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.

Sign up for GitHub

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

fix: add node-fetch v2 for script automation and keep node-fetch v3 #3038

Merged
merged 8 commits into from
Jul 17, 2024
Merged

fix: add node-fetch v2 for script automation and keep node-fetch v3 #3038

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
cd6086a
fix(automation-script): rollback node-fetch from v3 to v2
jerensl Jun 11, 2024
a24529e
fix(automation-script): add two version of node-fetch v3 and v2
jerensl Jun 11, 2024
25347e4
fix(automation-script): update lockfile and implement the node-fetch-2
jerensl Jun 11, 2024
0fb1af2
Merge branch 'master' into fix/automated-list-meeting-and-others
jerensl Jun 11, 2024
17523a0
Merge branch 'master' into fix/automated-list-meeting-and-others
anshgoyalevil Jun 26, 2024
d1aba21
Merge branch 'master' into fix/automated-list-meeting-and-others
jerensl Jul 15, 2024
03b444a
Merge branch 'master' into fix/automated-list-meeting-and-others
asyncapi-bot Jul 17, 2024
1cf54f8
Merge branch 'master' into fix/automated-list-meeting-and-others
asyncapi-bot Jul 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
"next": "14.1.1",
"next-mdx-remote": "^4.4.1",
"node-fetch": "^3.3.2",
"node-fetch-2": "npm:node-fetch@^2.7.0",
Copy link
Member

Choose a reason for hiding this comment

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

why npm:node-fetch@^2.7.0 and not just node-fetch@^2.7.0 ?

also maybe you could also update the other script that uses node-fetch, so there will be no need for node-fetch-2?

or maybe best solution is just to update https://github.com/asyncapi/website/blob/master/.github/workflows/regenerate-meetings-and-videos.yml#L28 that rungs this script. v4 is using node 20 by default - wouldn't that fix the issue? using v20 instead of v16 of node?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current node-fetch v3 is being used by the website only supports ESM modules, so I try to keep 2 versions.

Another solution is to migrate to ESM from CommonJS, I don't like this idea unless we are using Deno or Webpack to bundle the script, the primary reason is by the end of the day webpack or another tool will always bundle ESM to CommonJS in production.

It reminds me of the old days when this pissed a lot of people including me and almost break the entire javascript ecosystem xD
https://gist.github.com/sindresorhus/a39789f98801d908bbc7ff3ecc99d99c

"postcss": "^8.4.35",
"prettier": "^3.3.0",
"react": "^18",
Expand Down
2 changes: 1 addition & 1 deletion scripts/build-newsroom-videos.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const { writeFileSync } = require('fs');
const { resolve } = require('path');
const fetch = require('node-fetch')
const fetch = require('node-fetch-2')
async function buildNewsroomVideos() {

try {
Expand Down
Loading