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

feat: use node's native import() to load modules #2526

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AviVahl
Copy link
Contributor

@AviVahl AviVahl commented Jul 14, 2024

target is now node16, which means we still transpile our code to cjs (no "type": "module" yet).

we now retain the dynamic import() calls as-is, rather than transpiling them to require() calls.

this will make it easier for user projects to move to native esm.

@AviVahl AviVahl requested a review from barak007 July 14, 2024 15:42
@AviVahl AviVahl force-pushed the avi/use-node16-module branch 2 times, most recently from 57366c5 to 24caf98 Compare July 16, 2024 19:19
@barak007
Copy link
Collaborator

BTW
I don't like the way you squash locally and force push.
it makes all the comment notifications (and their comments) 404, and also I don't see what you are struggling with.

@barak007 barak007 marked this pull request as draft July 18, 2024 13:30
@AviVahl AviVahl force-pushed the avi/use-node16-module branch 4 times, most recently from 7553cb4 to 516ed0c Compare August 15, 2024 15:11
@AviVahl AviVahl force-pushed the avi/use-node16-module branch 2 times, most recently from 9da3810 to 8f67a3d Compare August 25, 2024 13:02
@AviVahl
Copy link
Contributor Author

AviVahl commented Aug 25, 2024

This didn't land as using dynamic import requires registration of a loader if you run directory from sources.
We do such thing downstream, so hold off until we have loaders in place.

@AviVahl AviVahl force-pushed the avi/use-node16-module branch 2 times, most recently from eae54db to 75fb030 Compare September 9, 2024 22:57
target is now node16, which means we still transpile our code to cjs (no "type": "module" yet).

we now retain the dynamic import() calls as-is, rather than transpiling them to require() calls.

this will make it easier for user projects to move to native esm.
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.

2 participants