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

Subsequent dynamic import of the same external module result in erroneous substitution of resolution namespace #2487

Closed
usergenic opened this issue Oct 1, 2018 · 4 comments · Fixed by #2505

Comments

@usergenic
Copy link

usergenic commented Oct 1, 2018

  • Rollup Version: 0.66.2
  • Operating System (or Browser): n/a
  • Node Version: 9.11.2 and 10.9.0

How Do We Reproduce?

Consider the following files:

// app.js
export function loadPage() {
  if (someCondition()) {
    import('./some-module.js');
  }
  if (someOtherCondition) {
    import('./some-module.js');
 }
}
// some-module.js
console.log('I have been loaded.');

I don't want rollup to inline any of the dynamically imported components, so I provide the fully resolved path to them in my as external option:

$ rollup app.js --output.format esm --external /resolved/path/to/some-module.js

Expected Behavior

// app.js
function loadPage() {
  if (someCondition()) {
    import('./some-module.js');
  }
  if (someOtherCondition) {
    import('./some-module.js');
 }
}

var app = /*#__PURE__*/Object.freeze({
  loadPage: loadPage
});

export { loadPage };

Actual Behavior

The first instance of the dynamic import of some-module.js leaks the local path name in the rolled up result. I think this called out by #2481

However, the second instance of the dynamic import of some-module.js is replaced with a promise returning the resolution namespace of app which is totally bogus.

// app.js
function loadPage() {
  if (someCondition()) {
    import("/Users/brendanb/src/github.com/usergenic/rollup-dynamic-import-problem/some-module.js");
  }
  if (someOtherCondition) {
    Promise.resolve().then(function () { return app; });
 }
}

var app = /*#__PURE__*/Object.freeze({
  loadPage: loadPage
});

export { loadPage };

💔

@usergenic
Copy link
Author

Sidebar: Is there any way to simply disable everything to do with rollup's treatment of dynamic imports and just leave them as-is? Have been poking at this from lots of angles but am failing to get it to do that.

@lukastaegert
Copy link
Member

However, the second instance of the dynamic import of some-module.js is replaced with a promise returning the resolution namespace of app

Are you using experimentalCodeSplitting? Inlining of dynamic imports is the default if this option is not used, which is what you observed here. The rest needs further investigation.

Switching off the handling of dynamic imports is not really possible at the moment but there is a poorly documented plugin hook that may help you: resolveDynamicImport

@usergenic
Copy link
Author

For now I am able to avoid this particular problem using Rollup 0.64.1

I played with resolveDynamicImport but I didn't seem to be able to avoid this behavior when using it.

@lukastaegert
Copy link
Member

Fix for the first issue at #2505. As for the second issue, see my comment above.

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 a pull request may close this issue.

2 participants