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

refactor: update fetch import for node v21+ compatibility #2573

Merged

Conversation

avallete
Copy link
Contributor

@avallete avallete commented Apr 19, 2024

Description

  • Use global fetch first, fallback to cross-fetch if needed
  • Addresses deprecation warnings due to unresolved issues in cross-fetch, node-fetch, and whatwg-url libraries
  • Ensures compatibility with environments running node.js v21+ where punycode module is deprecated

Related Issue

I've been following those issues on node-fetch node-fetch/node-fetch#1793
And on cross-fetch lquixada/cross-fetch#177

Both seems stale, so I hope that we can fix quicktype-core in the meantime. I've used the solution recommended into the cross-fetch issue toward conditional import and using native node fetch if available.

Motivation and Context

We're using quicktype-core in some of our tools, one of them being a cli tool, every-time a command is ran the deprecation coming from quicktype-code cross-fetch dependency shows up on node >=v21.

Previous Behaviour / Output

(node:6629) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)

New Behaviour / Output

No warning output.

How Has This Been Tested?

  1. Build and pack the quicktype-core package locally with the change
  2. Change my project quicktype-core dependency to point to the local builded package tgz and install
  3. Used FetchingJSONSchemaStore in node scripts to trigger the fetch behaviour with node versions 16.20.1, 18.20.2 and 21.7.3
  4. Build my project cli tool, and run it over with node versions v16.20.2, v18.20.2, v21.7.3 expected behaviour occurs and the warning is gone.

Screenshots (if appropriate):

- Use global fetch first, fallback to cross-fetch if needed
- Addresses deprecation warnings due to unresolved issues in
  cross-fetch, node-fetch, and whatwg-url libraries
- Ensures compatibility with environments running node.js v21+
  where punycode module is deprecated
@dvdsgl dvdsgl enabled auto-merge (squash) April 19, 2024 21:41
@dvdsgl
Copy link
Member

dvdsgl commented Apr 19, 2024

Thank you! Warnings be gone!

@dvdsgl dvdsgl merged commit 8d4169b into glideapps:master Apr 19, 2024
2 of 23 checks passed
inferrinizzard added a commit to inferrinizzard/quicktype that referenced this pull request Apr 28, 2024
inferrinizzard added a commit to inferrinizzard/quicktype that referenced this pull request Apr 28, 2024
dvdsgl pushed a commit that referenced this pull request Apr 28, 2024
* cast response.body to node readable stream for url fetch

* add comment

* Revert "refactor: update fetch import for node v21+ compatibility (#2573)"

This reverts commit 8d4169b.

* Revert "Revert "refactor: update fetch import for node v21+ compatibility (#2573)""

This reverts commit df1321d.

* add FIXME comment

* only use cross-fetch in CI
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

Successfully merging this pull request may close these issues.

None yet

2 participants