jaredpalmer / tsdx

Zero-config CLI for TypeScript package development
https://tsdx.io
MIT License
11.22k stars 507 forks source link

tsconfig target option is ignored -- use browserslist for Babel instead #951

Closed thany closed 3 years ago

thany commented 3 years ago

Current Behavior

Modern code is not getting transpiled to the target specified in tsconfig.json.

Expected behavior

It should respect the target option and transpile to that.

Suggested solution(s)

I dunno, if target is undiserable or whatever, maybe respect browserslist in package.json? Because that it also doesn't do. Otherwise it should just pick up target in tsconfig.json. Or I guess it should pick up something, anything, in order to make transpilation happen to a target of my choosing.

Additional context

Currently it does only very moderate transpilation, for instance a ?? b is transpiled, but { ...a, ...b } is not. Even setting target to a ridiculous es3 doesn't transpile any more (or less) than setting it to esnext. There should be some difference between the two, wouldn't you say?

So it is transpiling to some kind of target, but which one?... I don't know, but not my target, that's for sure.

Your environment

  System:
    OS: Windows 10 10.0.19042
    CPU: (12) x64 Intel(R) Core(TM) i9-8950HK CPU @ 2.90GHz
    Memory: 10.66 GB / 31.76 GB
  Binaries:
    Node: 12.20.0 - C:\Program Files\nodejs\node.EXE
    npm: 6.14.9 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Chrome: 87.0.4280.88
    Edge: Spartan (44.19041.423.0), Chromium (87.0.664.66)
    Internet Explorer: 11.0.19041.1
  npmPackages:
    tsdx: 0.14.1 => 0.14.1
    typescript: 3.8.3 => 3.8.3

Why are these scripts never seeing my Firefox browser? 🤨 Anyway, maybe it also helps to see my tsconfig.json:

{
  "include": ["src", "types"],
  "compilerOptions": {
    "target": "es5",
    "module": "esnext",
    "lib": ["dom", "esnext", "es2017"],
    "importHelpers": true,
    "declaration": true,
    "downlevelIteration": true,
    "sourceMap": true,
    "rootDir": "./",
    "strict": true,
    "noImplicitAny": true,
    "strictNullChecks": true,
    "strictFunctionTypes": true,
    "strictPropertyInitialization": true,
    "noImplicitThis": true,
    "alwaysStrict": true,
    "noUnusedLocals": true,
    "noUnusedParameters": true,
    "noImplicitReturns": true,
    "noFallthroughCasesInSwitch": true,
    "moduleResolution": "node",
    "baseUrl": "./",
    "paths": {
      "*": ["src/*", "node_modules/*"]
    },
    "jsx": "react",
    "esModuleInterop": true
  }
}

To the question "why do you need this?" I can answer simply: because I need to test stuff in this library on old browsers. Maybe libraries should not be transpiled. Maybe they should be. But that is an opinion not quite relevant to this issue. Just wanted to get that off the tables.

agilgur5 commented 3 years ago

Yes, TSDX indeed ignores tsconfig.json#compilerOptions.target; it transpiles to esnext via the TS compiler, then runs Babel, as you can see in the source code here:

https://github.com/formium/tsdx/blob/462af2d002987f985695b98400e0344b8f2754b7/src/createRollupConfig.ts#L176-L177

That comment was written by me as I also learned that the hard way in https://github.com/formium/tsdx/issues/413#issuecomment-570827672 / https://github.com/formium/tsdx/pull/130#issuecomment-569375532 / https://github.com/formium/tsdx/pull/415 .

I removed the option from the templates entirely in https://github.com/formium/tsdx/pull/466 (where it was even more confusingly set to es5) but I agree there should be more explicit prompting to the user when these are encountered. I recently added comments to all template tsconfigs in https://github.com/formium/tsdx/pull/864 (realized in https://github.com/formium/tsdx/issues/483 and #489 that you can add comments in tsconfig.json) so it might be good to add back in target: "esnext" to the templates with a comment that it will be overridden. In https://github.com/formium/tsdx/issues/760#issuecomment-692979270 I mentioned logging warnings for things that can't actually be configured, which is something that should definitely be done.

maybe respect browserslist in package.json?

TSDX uses @babel/preset-env by default which uses browserslist by default. So it should work with whatever configuration the version of browserslist allows. I manually tested this with .browserslistrc in https://github.com/formium/tsdx/issues/655#issuecomment-695843723 and confirmed it worked, but do still need to write an integration test for that per that comment. It was mentioned in the v0.14.0 release notes as well.

Not sure why browserslist in package.json didn't work for you, possibly a versioning thing or maybe some babelrc changes, would need more of a repro to test. But I'd recommend trying one of the other configuration options since at least one of them definitely works for me.

ruanyl commented 3 years ago

@agilgur5 Adding .browserslistrc seems NOT working for me, the *.cjs.production.js is ok which is properly transpiled. But *.cjs.development.js is not, I can see things like a ?? b