-
Notifications
You must be signed in to change notification settings - Fork 7
Support Node.js 7 and higher #11
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
Conversation
aacff5b
to
36f6ba6
Compare
Can we add tests in GitHub actions for different Node versions? |
@aciddjus Yes, that’s possible. Given we’ve manually tested it in our pairing session, what do you think if we add automated tests in a separate PR? |
Since tests would be directly related to what the PR is addressing let's have them here. |
4b94bb7
to
96311e7
Compare
I've modified the approach to handle polyfills. Now if the runtime does not have native implementations of the functions/classes (i.e. It opens up the possibility of supporting new runtimes that do not support most Node.js APIs such as Vercel Edge functions and Cloudflare Workers: #12 I've also temporarily disabled the home depot tests as that API is failing. |
9d150bc
to
5889096
Compare
5889096
to
c2db13f
Compare
This reverts commit 43fc759.
We are not targeting browsers, we don't need polyfills for them. This also removes extra dependencies, which improves compatibility.
d084de7
to
c29e4bd
Compare
b42ac6f
to
7cdf8c5
Compare
91e08c1
to
8c2f0d9
Compare
8c2f0d9
to
8d37edb
Compare
As we are not targeting browsers, we do not need to polyfill the missing APIs. The new commits by me have removed all the dependencies for the nodejs package. Built-in Node modules is not introducing new deps on the deno side either. |
05a4bed
to
6651ddd
Compare
6651ddd
to
16d4113
Compare
16d4113
to
545da21
Compare
e8ba809
to
99fc921
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @zyc9012 💯
LGTM!
Tested with the following Node.js versions. These versions were installed using
nvm
(e.g.nvm install 7
).v7.10.1
v8.17.0
v9.11.2
v10.24.1
v11.15.0
v12.22.12
v13.14.0
v14.21.1
v15.14.0
v16.19.1
v17.9.1
v18.14.1
v19.7.0
Changes
Most of the changes are related to removing the use of modern node features or by relying on a polyfill instead.
AbortSignal.timeout(timeout)
with a setTimeoutObject.fromEntries
with a for-loopfetch
fromcross-fetch@3.1.4
(rather than Node's in-builtfetch
)globalThis
,URL
,URLSearchParams
usingcore-js-pure@3.28.0
Misc changes
Related to #6