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

Chore: update dependencies to remove punycode depricated package #1755

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

DmitryAnansky
Copy link
Contributor

@DmitryAnansky DmitryAnansky commented Oct 8, 2024

What/Why/How?

In order to eliminate punycode deprecated warning we need to switch from node-fetch package that has it as a transitive dependency.

$ NODE_OPTIONS='--trace-deprecation' redocly lint
(node:48436) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
    at node:punycode:3:9
    at BuiltinModule.compileForInternalLoader (node:internal/bootstrap/realm:392:7)
    at BuiltinModule.compileForPublicLoader (node:internal/bootstrap/realm:328:10)
    at loadBuiltinModule (node:internal/modules/helpers:101:7)
    at Module._load (node:internal/modules/cjs/loader:1001:17)
    at Module.require (node:internal/modules/cjs/loader:1235:19)
    at require (node:internal/modules/helpers:176:18)
    at Object.<anonymous> (/opt/homebrew/lib/node_modules/@redocly/cli/node_modules/whatwg-url/lib/url-state-machine.js:2:18)
    at Module._compile (node:internal/modules/cjs/loader:1376:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1435:10)

It was decided to move to undici as it has full functionality and supports proxyAgent out of the box, comparing to native fetch (available from node version 18).
But it comes at a price - this is going to be breaking change as requires to rise minimal nodeJS version to

"node": ">=18.17.0",

Screenshot 2024-10-08 at 15 56 49

Reference

Closes: #1332

Testing

  • openapi-vscode

Screenshot 2024-10-08 at 17 54 08
Screenshot 2024-10-08 at 17 55 21
Screenshot 2024-10-08 at 17 55 33

  • monorepo
  • published portal verdaccio version in Reunite
  • workflows

Screenshots (optional)

Check yourself

  • Code changed? - Tested with redoc/reference-docs/workflows (internal)
  • All new/updated code is covered with tests
  • New package installed? - Tested in different environments (browser/node)

Security

  • Security impact of change has been considered
  • Code follows company security practices and guidelines

Copy link

changeset-bot bot commented Oct 8, 2024

🦋 Changeset detected

Latest commit: 1f397dc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@redocly/openapi-core Minor
@redocly/cli Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

github-actions bot commented Oct 8, 2024

Command Mean [ms] Min [ms] Max [ms] Relative
redocly lint packages/core/src/benchmark/benches/rebilly.yaml 974.1 ± 19.4 952.0 1017.0 1.00
redocly-next lint packages/core/src/benchmark/benches/rebilly.yaml 993.3 ± 21.9 965.2 1034.7 1.02 ± 0.03

Copy link
Contributor

github-actions bot commented Oct 8, 2024

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements 78.57% 4996/6359
🟡 Branches 67.2% 2065/3073
🟡 Functions 72.98% 824/1129
🟡 Lines 78.86% 4714/5978
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🟢
... / fetch-with-timeout.ts
91.67% 100% 50% 91.67%

Test suite run success

809 tests passing in 121 suites.

Report generated by 🧪jest coverage report action from 1f397dc

@DmitryAnansky DmitryAnansky changed the title Chore/update dependencies to remove punycode depricated package Chore: update dependencies to remove punycode depricated package Oct 8, 2024
},
ProxyAgent: (new (arg0: string) => any) | null;

if (typeof window === 'undefined') {
Copy link
Member

Choose a reason for hiding this comment

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

this is has to be fixed very carefully

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its for browser support, I found some logger tests for that.
Going to test in vscode / monorepo with some verdaccio package.

Copy link
Contributor Author

@DmitryAnansky DmitryAnansky Oct 8, 2024

Choose a reason for hiding this comment

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

I changed it to isBrowser from .env. Or should we use some other workaround?
It is more strict but similar:

export const isBrowser =
  typeof window !== 'undefined' ||
  typeof process === 'undefined' ||
  (process?.platform as any) === 'browser'; // main and worker thread
export const env = isBrowser ? {} : process.env || {};

@@ -1,555 +0,0 @@
{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tatomyr
This package-lock looks obsolete in the mono-repo setup, but please tell me if there is some idea behind it.

Copy link
Contributor

Choose a reason for hiding this comment

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

You can try removing it and see what happens.

@DmitryAnansky
Copy link
Contributor Author

DmitryAnansky commented Oct 11, 2024

Going to test another approach with native fetch => #1763

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.

punycode deprecation warning in node 21
3 participants