Skip to content

deps: qs@6.10.3 - #448

Merged
dougwilson merged 1 commit into
expressjs:masterfrom
ljharb:qs
Apr 3, 2022
Merged

deps: qs@6.10.3#448
dougwilson merged 1 commit into
expressjs:masterfrom
ljharb:qs

Conversation

@ljharb

Copy link
Copy Markdown
Contributor

I used a ^ here, since it's a very universal best practice to allow consumers to automatically get the latest bugfixes and security patches.

(I otherwise modeled this after a0ac3e9, which is unreleased since March 2020)

I'll remove the ^ if needed; either way I'm hoping for a rapid release so our app can dedupe the multiple copies of qs in the tree that we have due to body-parser not using a range for qs.

@ljharb

Copy link
Copy Markdown
ContributorAuthor

Note that the node 0.8 tests will fail unless you're using a later version of npm that understands ^. If that's an issue, I could change it to ~, but i doubt anyone's using an npm that lacks that understanding anymore.

@ljharb

Copy link
Copy Markdown
ContributorAuthor

(you also may want to add fail-fast: false in the matrix so it always runs every job even if one fails)

@dougwilson

Copy link
Copy Markdown
Contributor

Hi @ljharb sorry I didn't respond right away; when you first opened the PR and I looked, I noticed that this repo was still on the defunct Travis IC org system. I got moved over to GitHub Actions so the CI is running 🎉 .

It does look like unfortunately this version of qs cannot be released on the 1.x line, as it has a dependency that will no loner install with the default npm version that comes with Node.js 0.8 due to the ^ specified, as you have observed. The good news is that the body-parser 2.x line removed those old Node.js support and so I can package this upgrade in to that for you. A beta of the 2.0 needs to be released anyway, so that would be a good way to try it out.

@ljharb

Copy link
Copy Markdown
ContributorAuthor

As long as express 4 can update to body-parser 2, that'd be great.

Note that node 0.8, under any version of npm that works on it, can no longer install anything from the public registry ever again due to TLS changes - and the workaround of installing on 0.10 and switching back to 0.8 supports ^, so I'm not sure there's any reason anymore to worry about avoiding caret ranges.

@dougwilson

Copy link
Copy Markdown
Contributor

Hi @ljharb , hmm. Unfortunately express 4 cannot upgrade to body-parser 2.x as it contains backwards-breaking changes. As far as the state, I'm really not sure what our consumer are doing. Without being able to know who is installing from what, we need to adhere to semver and release breaking changes as major versions. Breaking install support from declared support would be a breaking change for consumers, whoever they are.

@ljharb

ljharb commented Nov 14, 2021

Copy link
Copy Markdown
ContributorAuthor

Totally agree; I'm just skeptical that there's any install graphs left, anywhere, where there's no carets. I've released carets in all my packages in semver-minors years ago and received zero reports of breakage, on a very large install base.

I could release a version of qs that uses ~ solely so body-parser 1 can pull it in, if that would do it.

@dougwilson

Copy link
Copy Markdown
Contributor

I could release a version of qs that uses ~ solely so body-parser 1 can pull it in, if that would do it.

I mean, that is up to you :) I would say that would be sweet, but at the same time, I don't wan to put that kind of burden on you. I know how it can be to do a bunch of stuff. I am trying to push out all new majors across the board and drop all these old Node.js's along with it; I have a whole bunch of vacations schedules that is really just me working on OSS haha.

How much trouble would that be, though? I looked at it seems that the one dep has it's own tree with ^ as well, so I feel like that would end up being a lot of trouble. Since I haven't actually released any 2.x alpha/beta/rc yet, I could potentially just dump the single 0.8 and 2.0 that for this module (since express 4 is 0.10). I'll have to take a look there for that idea.

@ljharb

ljharb commented Nov 14, 2021

Copy link
Copy Markdown
ContributorAuthor

That would be great (a v2 that only drops 0.8, and uses carets, that express v4 updates to use).

You’re right that because of the transitive carets, it’d likely be too much trouble to cut special releases; I didn't think of that.

@dougwilson

Copy link
Copy Markdown
Contributor

Hi @ljharb sorry for the lack of direcr update; many times other demanding users use up my "comment quota" and I just get tired of interacting for the day. I thnk you for your patience. I am working on what we discussed above: I will get a 2.0 of body-parser out with this and have determined a plan to get it in express 4.x as well without breaking folks, so yay! This week is the US Thanksgiving holiday, so I may be a bit slowed down, but this is my main priority for open source time until it is out

@dougwilsondougwilson self-assigned this Nov 21, 2021
@dougwilsondougwilson mentioned this pull request Nov 21, 2021
@ljharb

Copy link
Copy Markdown
ContributorAuthor

Yay thanks! Much appreciated. No rush; enjoy the holiday.

@ljharbljharb changed the title deps: qs@6.10.1deps: qs@6.10.2Dec 6, 2021
@ljharbljharb changed the title deps: qs@6.10.2deps: qs@6.10.3Mar 20, 2022
@dougwilson

Copy link
Copy Markdown
Contributor

I saw you just changed the title :) As a follow up, I'm just going to bump the npm dep to 1.2.8000 and land this in the 1.x branch. I mean, with the npm SSL changes, Node.js 0.8 is practically unusable anyhow, but it's also hard to use it without that "magic" npm (plus there is a deep dependency with that req already).

@ljharb

Copy link
Copy Markdown
ContributorAuthor

Awesome, totally agree!

@dougwilson
dougwilsonforce-pushed the master branch 3 times, most recently from 0ad1d88 to 2a2f471CompareApril 3, 2022 00:47
@dougwilson
dougwilson merged commit 77bcc0e into expressjs:masterApr 3, 2022
@ljharb
ljharb deleted the qs branch April 3, 2022 01:23
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ljharb@dougwilson