-
-
Notifications
You must be signed in to change notification settings - Fork 160
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ES Module conversion #92
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As long as this can be done without dropping support for so many Node.js versions. This PR doesn't even support all the active LTS versions of Node.js, which is 8.x (https://nodejs.org/en/about/releases/)
Maintenance for node 8 will end by next year, until then, we should release under alpha/beta versions only. The latest stable version of node is 10. |
Sure, but I was just using that as an example. I use this module myself down to Node.js 0.10. So accepting a PR that doesn't at least support down to there means I cannot even use my own module, which seems backwards. You're always welcome to publish your own module to support whatever Node.js versions you will use it on, though. Let me know if you are interested in bringing back support for those old versions, otherwise we can probably close this as won't fix. |
No problem! I'll close. Thanks for considering. |
Thanks for the PR, of course. I'm always happy to consider. It sucks if there is no way to support both the ES module folks and the CommonJS module folks, though. I don't mind adding features / whatever to the module as long as I can actually still use the module :) |
Yes, it sucks very much. I have tried multiple times, even once within the last month, to setup a easy to maintain system for supporting both, and right now I think the only reasonable way is to maintain a fork and publish two packages. There are solutions out there trying to be better than this, but they are crazy complicated and I would not want to maintain a package using them. |
@mkay581 I just saw your other comment and it looks like this is the approach you are taking. I think that it should not be too difficult to land any patches that land here on your fork. So that might help others looking for this support, thanks! |
Thanks @wesleytodd! Yes, unfortunately we need to fork this. I successfully got cookie-esm working in the 1.0 version I just released. Please feel free to redirect any ESM inquiries over to that repository. |
This converts the entire package to an ES Module and adds TypeScript support. ES Modules are the way forward and gives consumers a ton of benefits.
I realize that this may be a little too modern for many consumers who use very older versions of node, but we can possibly release this as an alpha release first to allow consumers to test it out before fully committing.
Please let me know your thoughts!
Fixes #87