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

Replace deprecated querystring component with builtin URLSearchParams API #416

Open
foosbar opened this issue Nov 12, 2021 · 8 comments
Open

Comments

@foosbar
Copy link

foosbar commented Nov 12, 2021

  • Operating System: macOS 11.6.1
  • Node Version: 16.13.0
  • NPM Version: 8.1.0
  • webpack Version: 5.64.0
  • ${package} Version: 2.25.1

Expected Behavior / Situation

Installing dependencies (npm install) for my project produces a deprecation warning for the querystring dependency.

npm WARN deprecated [email protected]: The querystring API is considered Legacy. new code should use the URLSearchParams API instead.

Actual Behavior / Situation

Remove the warning by removing querystring as a dependency and using URLSearchParams API

Modification Proposal

  • Remove the querystring component from the project
  • Set the required minimum version of Node.js to >= 10.12.0 (picked this version since mocha has the same requirement - but Node.js 10 supports the full URLSearchParams API)
  • Update client.js to utilize this new API

The big question is can we limit webpack-hot-middleware to Node.js 10.12 or higher - webpack@5 requires 10.13 or higher.

@foosbar foosbar changed the title Replace deprecated querystring component with builtin URL Replace deprecated querystring component with builtin URLSearchParams API Nov 12, 2021
@glenjamin
Copy link
Collaborator

glenjamin commented Nov 12, 2021

The deprecation warning on that npm package is actively harmful imo.

The querystring API is considered Legacy. While it is still maintained, new code should use the API instead.

There is nothing about this message that suggests there is any real reason to stop using the existing API which has been stable for years.

That said, there iss an open PR for this already #411

@foosbar
Copy link
Author

foosbar commented Nov 12, 2021

Agreed @glenjamin, but deprecated dependencies typically stop getting support and using standard APIs for such a minor usecase would offer better long term support.

Thanks for pointing out the open PR - missed that one and was about to open another.

@glenjamin
Copy link
Collaborator

using standard APIs for such a minor usecase would offer better long term support.

Yeah, if this was a random 3rd party module I'd be more inclined to switch, but querystring has been a standard and stable part of Node.js since at least 0.10.x in 2013, if not longer!

I suspect that npm warning is annoying enough that it's going to be easier to merge the PR than to leave things as they are

@dartess
Copy link

dartess commented Aug 10, 2022

@glenjamin hey! friendly ping:

That said, there iss an open PR for this already #411

it's going to be easier to merge the PR than to leave things as they are

are there any plans or barriers to merge that PR?

@glenjamin
Copy link
Collaborator

The only barrier right now is that I never got around to it, and I really really really dislike that they've marked a core library as actively deprecated rather than just frozen.

There is literally nothing wrong with the current code - and it annoys me that someone has created this work for any maintainer that depends on querystring.

@dartess
Copy link

dartess commented Aug 10, 2022

@glenjamin yeah, it's disappointing. I understand.

Contrariwise I think I can understand the author of that package. querystring is based on the corresponding module from Node.js. And he, in turn, is legacy now (https://nodejs.org/api/querystring.html). Thus, the package continues to keep parity with its original. Or maybe I just made it up myself.

Anyway, on the other hand, we are users of these packages who receive annoying warning that distract from the really important warnings.

@glenjamin
Copy link
Collaborator

Yeah, I should have some time tomorrow to take a look and get this sorted

Thanks for the prompt

@glenjamin
Copy link
Collaborator

This is now released in 2.25.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants