yargs / y18n

:ledger: the bare-bones i18n library used by yargs
ISC License
146 stars 48 forks source link

fix: entrypoint specified in `module` #156

Open tasshi-me opened 1 year ago

tasshi-me commented 1 year ago

This PR fixes https://github.com/yargs/y18n/issues/155.
I made the entrypoint specified in the module field the same as exports.

shadowspawn commented 1 year ago

As a sanity check I looked at what some of the other Yargs libraries have, and import entry point and "module" do match each other.

https://github.com/yargs/yargs-parser/blob/3aba24ceaa1a06ceb982c63a06002526d781e826/package.json#L6-L19

https://github.com/yargs/cliui/blob/af3145da0ea31738c4715865a6da0ee388a94c74/package.json#L6-L16

shadowspawn commented 1 year ago

I note you put fix! for a breaking change in the title. Do you feel it is a breaking change, or is it just a bug fix?

(I don't have much experience with bundlers and interested in your reasoning.)

tasshi-me commented 1 year ago

@shadowspawn Thank you for reviewing! You can remove !.

I added it because this PR may break environments using older versions of bundlers that don't support the exports field. I don't know how many users use this package directly and are impacted.

I checked when webpack and rollup started supporting exports.

shadowspawn commented 1 year ago

Thanks for info, and good links.

I found a couple of links covering background and usage of the "module" main field:

tasshi-me commented 1 year ago

@shadowspawn

Thank you for letting me know!

Now, my idea...

  1. Release with fix, because it changes the interface for old bundlers, but the user impact is small
  2. Release with fix!, even if user impact is small
    1. I think it's nice to drop Node 10 and 12 on the same major update.

I respect your decision.