Skip to content
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

Release 2.0 #66

Merged
merged 24 commits into from
Sep 10, 2024
Merged

Release 2.0 #66

merged 24 commits into from
Sep 10, 2024

Conversation

dougwilson
Copy link
Contributor

@dougwilson dougwilson commented Oct 30, 2014

This is a tracking issue for release 2.0.

I am trying to give better visibility for upcoming changes and so am trying out making a PR for a release here, pulling from the official next release branch into master. This allows for the current pending changes to be easily visible.

Please keep feature requests in their own issues

I'm also leaving this PR unlocked so people can make comments/etc. and we'll see how it goes :) If you want to make a comment on a particular change, please make the comment in the "Files changed" tab so comments are not lost during a rebase.

List of changes for release:

List of deprecated things removed:

  • Main bodyParser() function which did urlencoded + json

Testing this release

If you want to try out this release, you can install it with the following command:

$ npm cache clean
$ npm install expressjs/body-parser#2.x
$ npm cache clean

@Fishrock123
Copy link
Contributor

@dougwilson isn't abstraction from parsers or whatever scheduled-ish for 2.0?

@dougwilson
Copy link
Contributor Author

Yea, I haven't fully added everything to the checklist yet. I just wanted to make the PR so it starts to get eyes.

@LinusU
Copy link
Member

LinusU commented Aug 3, 2015

@dougwilson Would love to use this, have you thought about publishing to npm as 2.0.0-beta1? Thanks 👍

@dougwilson
Copy link
Contributor Author

Hey @LinusU ! I never published it because I didn't think there was anyone wanting to start, so without demand, I did not want to burden myself with maintaining two working release lines at once, if it wasn't necessary. I'll see about getting a alpha/beta out, though :) You can always add a branch/commit of a git repo as an npm dependency, though.

@LinusU
Copy link
Member

LinusU commented Aug 28, 2015

Cool. I mainly wanted the first bullet which caused a bug for me when only using the raw middleware. But I just worked around it in code since I still want to get bugfixes. Would be cool to get 2.0 out for real thought. Whats missing? Is there anything I can help with?

@theganyo
Copy link

theganyo commented Nov 6, 2015

Hey @dougwilson! So I just found this because of the bugs I've been battling today related to #128, but I see the issue and this PR have been open for over a year despite being fixed. Since it doesn't look like this is going to happen (or is there still a chance?), is the only option to fork and release under a different name?

@dougwilson
Copy link
Contributor Author

Hi @theganyo , you are certainly welcome to fork & release this under another module, as it is under the MIT license, but there are a few reasons why it has yet to be released:

  1. I had a report that the changes in here can sometimes stall out a request. I have not been able to look, but if you would like to test it out on your systems (using the instructions provided in the original post), that would be much appreciated!
  2. This was proposed over a year ago, but with Express 3.x still supported and relying on this module, it was not possible to release a breaking version without adding more work for myself, which is all done unpaid, and so I've been waiting for Express 3.x to go away first.

is the only option to fork and release under a different name?

You can always depend on the branch in GitHub in your package.json... https://docs.npmjs.com/files/package.json#git-urls-as-dependencies

@dougwilson
Copy link
Contributor Author

And since Express 3.x is now "officially unsupported", I want to just make one last 1.x release here and then publish a 2.0.0 alpha for people to start testing on.

@theganyo
Copy link

theganyo commented Nov 6, 2015

Thanks, Doug. As you might have guessed, forking and releasing isn't at top of my list of alternatives. :) I just wanted to gauge whether this was really still potentially going to be supported. Since you're looking at a new release, I'll just hack around it for now and update later. Thanks for the quick reply!

@wesleytodd wesleytodd mentioned this pull request Jul 22, 2024
4 tasks
papandreou and others added 2 commits July 24, 2024 16:38
* Always use the qs module, even in simple mode

#326 (comment)

* Create the simple and extended parser with the same function, reducing duplication

* Don't mention the querystring module in the README

* Fix lint
Copy link
Member

@UlisesGascon UlisesGascon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't release 2.0 before I approve this PR.

Based on a private discussion, I want to include a major change that has not been made public yet.

…326)

* urlencoded, in extended mode: Support iso-8859-1 encoded requests, and also accept iso-8859-1 as a default encoding.

* urlencoded: Support an utf8 sentinel to detect the charset.

* Pass the interpretNumericEntities option through to qs.

* Fix lint

* Support charsets, sentinels etc. via custom decoders

Works in both extended and simple mode.

* Simplify

* Fix empty parameter issue with utf8Sentinel in simple mode

* Run all the charset/sentinel tests in both extended and simple modes

* utf8Sentinel => charsetSentinel

ljharb/qs#268 (comment)

* Update qs to 6.9.1

* Always use the qs module, even in simple mode

#326 (comment)

* Create the simple and extended parser with the same function, reducing duplication

* Don't mention the querystring module in the README

* Fix lint

* Update qs to 6.9.4

* Consistently call it "utf8 sentinel"

* Simplify by relying on the qs module's support for detecting the charset
#326 (comment)

* Simplify further

* Put back debug option

* Document that defaultCharset defaults to utf-8
#326 (comment)
@UlisesGascon
Copy link
Member

Just to confirm @wesleytodd... only my change is missing for releasing it? Do you want to work on the conflicts?

@wesleytodd
Copy link
Member

I was going to work out the conflicts after updating all the deps and stuff. I think I looked at them when I tried to rebase this and it was enough questions I needed more time which I didn't have then. I was thinking that it would be easiest to do all that resolution at the same time right before publishing than doing it now and possibly having to do it again later.

@RobinTail
Copy link

I want to include a major change that has not been made public yet.

What's been holding you from doing that yet, @UlisesGascon?
3 weeks passed. This package is a blocker for Express 5.

@wesleytodd
Copy link
Member

Hey @RobinTail, I appreciate the energy to keep things moving. That said, there are things going on behind the scenes which you are likely not aware of. If you would like to help out with that work, please come to our working sessions or start helping with other efforts and then we can include you in the non-public discussions. In the mean time, please know that the delays here are 100% necessary and intentional.

danielgindi and others added 3 commits August 17, 2024 13:42
- Update documentation to reflect the new features and errors
- Update the changelog
- Upgrade to `qs@6.13.0`
- Add the `depth` option to define the depth of parsing while parsing the query string
- Enable the `strictDepth` option by default in `qs.parse`
- Add a 400 status code when the depth of the query string exceeds the limit defined by the `depth` option
- Reduce the default depth limit to 32
Copy link
Member

@UlisesGascon UlisesGascon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Finally we are releasing it ! 🥳

@wesleytodd wesleytodd merged commit a50ab74 into master Sep 10, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants