jaredpalmer / tsdx

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

(fix): --name shouldn't change output filenames #669

Open kotarella1110 opened 4 years ago

kotarella1110 commented 4 years ago

Fixed output file name after build to use the name of package.json instead of the passed name if the --name option is passed.

Currently, the output file name use the name passed with the --name option, as follows:

  1. Create a library using hyphen: npx tsdx create my-lib
  2. Build with --name option: yarn build --name MyLib --format esm,cjs,umd
  3. Check dist dir: tree dist/
dist/
├── index.d.ts
├── index.js
├── mylib.cjs.development.js
├── mylib.cjs.development.js.map
├── mylib.cjs.production.min.js
├── mylib.cjs.production.min.js.map
├── mylib.esm.js
├── mylib.esm.js.map
├── mylib.umd.development.js
├── mylib.umd.development.js.map
├── mylib.umd.production.min.js
└── mylib.umd.production.min.js.map

I think the output file name should use the name of package.json instead of the name passed with the --name option, as follows:

dist/
├── index.d.ts
├── index.js
├── my-lib.cjs.development.js
├── my-lib.cjs.development.js.map
├── my-lib.cjs.production.min.js
├── my-lib.cjs.production.min.js.map
├── my-lib.esm.js
├── my-lib.esm.js.map
├── my-lib.umd.development.js
├── my-lib.umd.development.js.map
├── my-lib.umd.production.min.js
└── my-lib.umd.production.min.js.map

In fact, in ReactDOM, it is react-dom.development.js, not reactdom.development.js.

agilgur5 commented 4 years ago

This is somewhere in between a bug fix and a behavior change/feature, because the docs and the help dialog say it's only for UMD names, but apparently it's used for the output filename in the source code too. I didn't realize that either and have touched all of this code (though these pieces weren't written by me), good catch!

I'll have to look at the history and possibly microbundle history to see if this is an evolution of the --name use-case or if it's a bug that it's used this way in the code.

In any case, this is a breaking change as there might be folks who use this undocumented behavior. There's another related request back in https://github.com/jaredpalmer/tsdx/pull/208#issuecomment-536625558 / #219 around changing the output name to always just be dist/index.js instead of this safePackageName stuff which I agree with, for consistency's sake. Changing that would fix this too, but that change is extremely breaking and this one not so much.

kotarella1110 commented 4 years ago

@agilgur5 Do you have any update on this?

agilgur5 commented 4 years ago

So I did some investigation and this indeed some likes an unintentional bug that --name changes the filename on top of the UMD name. The docs both here and in microbundle say it's just for UMD. microbundle has a very similar bug that will cause, in its case, options.name to be set to the --name flag, but it doesn't affect microbundle nearly as much because its filenames are set to whatever you've specified in your package.json fields. That might be a good feature for us to have, which would make any filename changes a bit less breaking (though itself would similarly be breaking if a user did any renames on their own). It was previously proposed in #219 as well.

That being said, because this is unfortunately a breaking change, this won't be able to go out soon. v0.14.0, the next minor (minors are breaking in v0.x), is already going to be a pretty big release with a handful of dependency-related potentially breaking changes, so this won't make it in there either 😕 (don't want to overload users with too many breaking changes at once) Probably won't be till at least v0.15, so I wouldn't hold out on this getting merged in soon unfortunately.

In the meantime, if you need this, I'd recommend using patch-package or configuring tsdx.config.js to get this behavior, sorry about the unfortunate timing / breaking-ness.


Also in the meantime, I'll still make some review comments and I'll give credit to you with allcontributors for this bug report and code, esp since some code (the removals) was already used in #671 .

agilgur5 commented 4 years ago

@allcontributors please add @kotarella1110 for bug, code

allcontributors[bot] commented 4 years ago

@agilgur5

I've put up a pull request to add @kotarella1110! :tada: