comment-parser icon indicating copy to clipboard operation
comment-parser copied to clipboard

Version 1.1.6-beta.2 was breaking

Open homer0 opened this issue 3 years ago • 4 comments

Hey 👋 !

First of all, great lib 😁, really useful, and I'm thankful for the effort you put in it 🤘.

Now, sorry to be that guy, but when you released 1.1.6-beta.2, while adding the compatibility with ESM and CJS, you introduced a breaking change, .js to .cjs, and people installing your lib with a range like ^1.1.5 automatically get the update to 1.2.3 and their implementations will break.

It's kind of late now, since you already released 5 versions after that, and I already fixed my implementation :P, but I wanted to let you know either way, you can close this issue.

Finally, I know is a pain to do the ESM and CJS stuff (I even made a lib for my libs), so the situation it's understandable, but I wanted to recommend you semantic-release + conventional commits... yes, it's not easy to get used to the commits format (although you can even use commitlint), but it's soooo worthy not to have to worry about the releases and the CHANGELOG.

I hope some of that was helpful! Again, thanks for the lib 🤘!

homer0 avatar Aug 02 '21 04:08 homer0

Quick update: when using it as a direct dependency, I had no issue with importing the .cjs, but when I used it as a dependency of a dependency (basically when I used my plugin in another project), I got this:

Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './lib/index.cjs' is not defined by "exports" in [...abs-path]/node_modules/comment-parser/package.json

No rush, I pinned the dep :D ... I really like ESM, but damn this transition is a pain, good luck!

homer0 avatar Aug 02 '21 05:08 homer0

@homer0 thanks for the feedback. You have a good point, it might be too late to fix anything now. It was my mistake blindly believing node/npm would transparently accept this transition. Will be cautious with this kind of changes in the future

will check out suggested tools, thanks

syavorsky avatar Aug 02 '21 19:08 syavorsky

Hi,

As the person responsible for the commits (and not having paid enough attention that this was being added to a patch version), I appreciated your understanding for the hiccups that are sometimes involved with a transition to ESM.

Quick update: when using it as a direct dependency, I had no issue with importing the .cjs, but when I used it as a dependency of a dependency (basically when I used my plugin in another project), I got this:

Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './lib/index.cjs' is not defined by "exports" in [...abs-path]/node_modules/comment-parser/package.json

FWIW, although the extension change did cause problems in this case, this would not have been a problem had the fix later added as part of 1.2.2 (37905fe35995fc04e46d2e73ea3115eda5017f8d ) been taken care of at that time.

The ESM changes were still technically breaking though for those who were reaching into submodules, but just wanted to mention this requiring of main was an issue due to an incomplete PR rather than that most users should be affected by a (proper) extension change per se.

brettz9 avatar Aug 14 '21 01:08 brettz9

Hi @brettz9

You are right, the problem was reaching for the sub paths rather than using the main; the thing is that the README tells you to use 'comment-parser/lib', and I'm sure I'm not the only one that used that snippet as a starting point.

Maybe that should be updated, to avoid this kind of problems.

Awesome job btw 🤘!

homer0 avatar Aug 14 '21 05:08 homer0