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

Update to support node v18+ #4

Merged
merged 2 commits into from
Aug 29, 2023
Merged

Update to support node v18+ #4

merged 2 commits into from
Aug 29, 2023

Conversation

jacksongross
Copy link
Contributor

This package doesn't seem to work in node v18+ and I suspect its due to how fetch is loaded within node-fetch-native. Considering that FormData and fetch are now natively supported in node from v18 we can remove these dependencies and with a few tweaks make this package work again.

These are natively supported from node v18+ and the previous code wouldn't work, so best to remove
these dependencies altogether.
…cies

They are no longer needed as they are now globals in Node from v18+.
filename: asset.original_file,
contentType: 'application/json'
})
form.set('access_token', accessToken)
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 main issue I was seeing within our apps using this plugin was that the form data was not being supplied to Rollbar when making the API call from within uploadSourcemap, with API responses indicating an access token was missing, and it was definitely defined within our environment so it lead me to find that the FormData was not sending the data correctly via fetch. Then I discovered the node-fetch-native package falls back to native fetch when defined, which doesn't seem compatible with the form-data package we are using, which lead me to find form-data/form-data#551 which seems it is not necessary anymore due to being a global in node.

@rang-ali
Copy link
Contributor

Thank you for your contribution!

@rang-ali rang-ali merged commit b5ea865 into clinggroup:main Aug 29, 2023
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