mattermost / mattermost-developer-documentation

Mattermost developer documentation.
https://developers.mattermost.com
BSD 3-Clause "New" or "Revised" License
80 stars 401 forks source link

Replaces node-fetch v2 (sync-fetch) with node-fetch v3 #1391

Closed tnir closed 4 months ago

tnir commented 4 months ago

Summary

This change replaces sync-fetch with await and node-fetch@3.3.2 as sync-fetch does not support node-fetch v3 and allow us to avoid whatwg-url@5.0.0. node-fetch@3.3.2 does NOT depend on any version of whatwg-url.

Ticket Link

n/a

Why

node-fetch 2.x, followed by sync-fetch, does not support Node.js 21 and higher fully as it relies on punycode depending from whatwg-url 5.0.0.

As is, with Node.js 21+, we can see warning as follows due to dependency to whatwg-url@5.0.0:

$ make plugin-data                
mkdir -p site/data
go run ./cmd/plugin-godocs > site/data/PluginGoDocs.json
go run ./cmd/plugin-manifest-docs > site/data/PluginManifestDocs.json
[...]
rm -rf scripts/mattermost-webapp || true
cd scripts && npm install

up to date, audited 45 packages in 535ms

[...]

mkdir -p site/data
node scripts/plugin-jsdocs.js > site/data/PluginJSDocs.json
(node:48006) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
(node:48005) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)

Refs.

mattermost-build commented 4 months ago

Hello @tnir,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

M-ZubairAhmed commented 4 months ago

@tnir you mentioned we can avoid whatwg-ur but what is the reasoning behind avoiding it?

tnir commented 4 months ago

@M-ZubairAhmed Thanks. I just added some explanation to the description. Thoughts?

M-ZubairAhmed commented 4 months ago

/update-branch

M-ZubairAhmed commented 4 months ago

@tnir Thank you for the explanation this makes sense to me

tnir commented 4 months ago

@M-ZubairAhmed Thanks. With your update the branch, it looks like being ready for merging to me.

M-ZubairAhmed commented 4 months ago

@M-ZubairAhmed Thanks. With your update the branch, it looks like being ready for merging to me.

Yes sure, lets ask @cwarnermm for a lookup

github-actions[bot] commented 4 months ago

Newest code from cwarnermm has been published to preview environment for Git SHA 73a47a2ce0ee0289622ab46be5e0791b8db6bdf5