Use "node:" prefixed imports everywhere #154
Conversation
boenrobot
force-pushed
the
runtimeCompat
branch
3 times, most recently
from
73a5006 to
947d0bc
Compare
…version to the ones that support this. CI is updated accordingly.
|
I know this is over a year later; my apologies, I must have somehow missed the notification for this. I'm not opposed to this change in principle; obviously is a breaking change. If we're going to release a major version, it probably makes sense to do other changes, like using |
| node: [10.x, 12.x, 14.x, 16.x] | ||
| runs-on: ubuntu-latest | ||
| node: [14.18.0, 14, 16.0.0, 16] | ||
| os: [ubuntu-latest, macOS-latest, windows-latest] |
There was a problem hiding this comment.
Appveyor is windows testing; no need to duplicate it here. Is there a good reason for testing macOS separately?
There was a problem hiding this comment.
File system related functions behave differently in some cases, even between Ubuntu and MacOS. I can't exactly enumerate said cases, as I don't know the details of when they happen or why...
I also am not sure if this package in particular is affected by them, but because it does touch the file system, it seems like it should be checking if it is affected. If you insist, I'll remove it, but IMO, it's better to include it for extra assurance.
As for Windows being duplicated between GH runners and AppVeyor... I actually submitted this PR as part of a batch to several packages, so I copy & pasted most things (except the imports, obviously), including this, and other packages don't use AppVeyor... If you prefer to use AppVeyor, sure, I'll remove that. But have you considered maybe not using AppVeyor, and using only GH runners instead? That would make all tests more "unified".
There was a problem hiding this comment.
No strong feelings on macOS testing. Migration from AppVeyor to GH Actions might be good, but would need to involve @jprichardson (I don't have necessary admin access), and shouldn't block this PR.
|
Needs rebase |
And bump the minimum nodejs version to the ones that support this.
CI is updated accordingly.
Using "node:" prefixed imports enables this library to be used with runtimes other than node which have a node compatibility layer when internal dependencies are prefixed with "node:". Case in point is
workerd. The cost is raising up the minimum node version to ">=14.18.0 <15 || >=16".