-
-
Notifications
You must be signed in to change notification settings - Fork 22
V3: ESM+CJS, automatic split combined cookies, & general API cleanup #74
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
base: master
Are you sure you want to change the base?
Conversation
… import styles, comments, etc.
dist/set-cookie.cjs
Outdated
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.
This is just a copy of of lib/set-cookie.js, but with the exports at then end changed to cjs.
|
I'm going to go ahead and release this in a few days if I don't hear back from anyone before then. |
|
I doesn't have the time to try sorry ... But, some things seems strange, why did you commit the "dist" ? Also, you check if git is not clean in CI ? ( strange ? why ? ) . |
That's a good point. I started doing this for TypeScript projects, because it's nice to be able to check out a git repo and use it without having to compile first, but for something like this, it's technically usable without compiling. Maybe I'll remove that for this one.
It actually does both! The
That's to make sure they don't get out of sync, but if I switch to only building in CI, then I can remove that check. |
I understand your point, and it's your project, so I respect your choice! It just feels a bit 'non-standard' to me. In most professional workflows I've seen, we usually do the opposite: the repo stays 100% source-only. Generally, users either fetch the compiled assets from a Release/NPM or build it themselves after cloning. Committing generated files is often avoided because it adds a lot of noise to pull requests and can lead to merge conflicts. |
This has a few big changes all rolled into one:
package.jsonto make it work in either type of project.splitCookieString()is now called automatically when appropriate, with a newsplitoption to override the behaviorparseSetCookie(input, options)I think this will be a non-breaking change for most, if not all, users. But I'm not 100% sure and I had previously stated that the automatic
splitCookieString()would be in a major version, so calling this 3.0 feels like the right move.@bytesnz, @thib3113, @realityking, @benmccann, @renatoaraujoc, & @LenweSaralonde would you each mind giving this a shot sometime in the next week or two? (Or let me know if you need more time than that.)
You can install this branch in your local project with this command:
npm install git+https://github.com/nfriedly/set-cookie-parser#v3Fixes #73
Fixes #50
Relates to #68