grammyjs / grammY

The Telegram Bot Framework.
https://grammy.dev
MIT License
2.28k stars 112 forks source link

DeprecationWarning: The `punycode` module is deprecated #519

Closed emmtte closed 9 months ago

emmtte commented 9 months ago

Warning Message

grammy@latest(node:10501) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.

KnorpelSenf commented 9 months ago

grammY does not depend on this package. Run npm ls punycode to find out why you installed it.

emmtte commented 9 months ago

Result is empty

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

npm ls punycode └── (empty)

KnorpelSenf commented 9 months ago

Do you get the same warning if you rm -rf node_modules package-lock.json and then npm install?

emmtte commented 9 months ago

Yes, same warning and npm ls punycode is empty

npm install

added 75 packages, and audited 76 packages in 36s

36 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities
emmtte commented 9 months ago

With yarn more infos

yarn install v1.22.19
[1/4] Resolving packages...
⠄ grammy@latest(node:14912) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
[2/4] Fetching packages...
[3/4] Linking dependencies...
[4/4] Building fresh packages...
success Saved lockfile.
Done in 34.37s.
KnorpelSenf commented 9 months ago

I cannot reproduce this. When I create a new project and install the latest version of grammY, there is no warning. Since we also don't depend on that module, I don't know what we can do here to fix it.

What happens when you uninstall grammY from the project? There are dozens of other packages that are unrelated to grammY. Does the issue persist?

KnorpelSenf commented 9 months ago

Closing due to inactivity. It's invalid anyway, we have nothing to do with this dep. If a plugin needs it and I'm not aware of that, please reopen an issue like this for the respective plugin page.

fend25 commented 8 months ago

The problem's because of node-fetch@2.7. Bumping node-fetch up to v3 should fix it.

KnorpelSenf commented 8 months ago

There is no way to bump it to v3. It is the plan of v3 to break all CJS projects. That would be a breaking change.

We're completely going to drop node-fetch for our v2 whenever that may happen. We're also never going to use a dependency authored by the responsible person ever again.

fend25 commented 8 months ago

Thank you for such a quick response!

Yes, I'm encountering issues with ESM modules in v3 while trying to create a PR here.

What do you think about this potential workaround in package.json?

  "resolutions": {
    "whatwg-url": "^14.0.0"
  }

Kinda hacky, and not mandatory at all. Just asking, not pushing:)


edited:

Ah, v13 dropped Node@14 support, v14 dropped Node@16 support. Unlucky.

KnorpelSenf commented 8 months ago

Ehh i have not really looked into that to be honest. Does your edit mean there's no possible way to fix this using resolutions? (If there was a way, I wouldn't mind using it to get rid of the deprecated packages.)

fend25 commented 8 months ago

I'm uncertain about how it will behave when installed as a dependency. When I add a resolutions block to the package.json, I do it at my own risk. However, I'm unsure if I should recommend such an approach for a library.

KnorpelSenf commented 8 months ago

I still don't really understand why this is happening, though.

cd /tmp
npm init -y
npm i grammy
npm ls -a

yields

tmp@1.0.0 /tmp
└─┬ grammy@1.21.1
  ├── @grammyjs/types@3.5.2
  ├─┬ abort-controller@3.0.0
  │ └── event-target-shim@5.0.1
  ├─┬ debug@4.3.4
  │ └── ms@2.1.2
  └─┬ node-fetch@2.7.0
    ├── UNMET OPTIONAL DEPENDENCY encoding@^0.1.0
    └─┬ whatwg-url@5.0.0
      ├── tr46@0.0.3
      └── webidl-conversions@3.0.1

so where exactly does punycode come from?

NullEnt1ty commented 6 months ago

so where exactly does punycode come from?

"punycode" is a Node.js core module and not a dependency that will be installed by npm. Therefore it will not be listed by npm ls.

The module is used by the dependency "whatwg-url": https://github.com/jsdom/whatwg-url/blob/d34854a7af6ed1204f55d7da7761497bad350a7b/src/url-state-machine.js#L2

Sorry for reviving this issue. I wanted to shed some light on this module and where it comes from. Although I fear that this will not help to solve the problem 😅

KnorpelSenf commented 6 months ago

Ah that explains it, thanks for the info! We're just going to solve it by dropping node-fetch for grammY 2.0, so this will be fixed eventually.

egoroof commented 4 months ago

I have the same issue with latest version of grammY. Why is this issue closed and marked "invalid"?

KnorpelSenf commented 4 months ago

It was closed because there was no reproduction example and the original author didn't respond. It's no longer accurate that it's invalid, so I removed the label, but we still can't do anything about this. It is useful to keep open those issues that actually can be fixed by us.

egoroof commented 4 months ago

but we still can't do anything about this

why?

KnorpelSenf commented 4 months ago

Because that would be a breaking change, which we can only do for v2

egoroof commented 4 months ago

would it be a breaking change to fork package node-fetch v2.7.0 and change node module punycode dependency to https://github.com/mathiasbynens/punycode.js and release v2.7.1 and use it in your grammY ?

KnorpelSenf commented 4 months ago

No, feel free to do that. I hope that the node-fetch maintainers will accept your changes.

It would be breaking if we would bump node-fetch to v3 because that removes compatibility with CJS and some old supported versions of Node. It would also be breaking to drop node-fetch entirely because we currently re-export the fetch options from it, so any replacement for node-fetch must ship with alternatives that are called exactly the same and that behave exactly the same, which is very unlikely for any library to be true.

Simply making a patch will work well, assuming that the change doesn't require a major version from node-fetch.

Xanders commented 3 months ago

Hi there! As a simple workaround while waiting for grammY 2.0, I can suggest adding this to the bottom of package.json:

{
  "overrides": {
    "whatwg-url": "^14.0.0"
  }
}

DISCLAIMER: I have no idea what it can break potentially, I didn't run grammY test suite, so use it at your own risk. In my bot with few commands and callbacks there seems no negative effects.