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

fix: add tests and better error for dynamic imports #39

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pi0
Copy link

@pi0 pi0 commented Sep 29, 2021

Reference: #30

  • Add failing test for dynamic imports
  • Add a more relevant error for dynamic imports

In order to support dynamic imports and implement importModuleDynamically, we have to use vm.SourceModuleText which is still experimental and only available behind --experimental-vm-modules flag. Yet current situation might be super confusing especially when users are not directly using v8-compile-cache and see a clueless error like this:

(node:8373) UnhandledPromiseRejectionWarning: TypeError [ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING]: A dynamic import callback was not specified.
    at exports.importModuleDynamicallyCallback (internal/process/esm_loader.js:34:9)

After this PR: (git.io link redirects to #30)

(node:8503) UnhandledPromiseRejectionWarning: Error: [v8-compile-cache] Dynamic imports are currently not supported. See https://git.io/Jge6z for more information.
    at importModuleDynamically (./home/pooya/Code/v8-compile-cache/v8-compile-cache.js:248:15)

I would be happy to also help in the future with the actual implementation of the VM cache 😊

@ocombe
Copy link

ocombe commented Nov 18, 2021

I wish this PR could be merged, I've spent a week trying to figure out why loading ESM modules dynamically didn't work... this would have saved me so many hours !

@pi0
Copy link
Author

pi0 commented Nov 18, 2021

/cc @zertosh I know you are probably super busy just pining to see if there is a chance this can be rolled out sometime soon 💚

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