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 dynamic import of modules with inlined imports #8648

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

Conversation

comp500
Copy link

@comp500 comp500 commented Nov 21, 2022

↪️ Pull Request

When dynamically importing a module that itself has an inlined import (e.g. with bundle-text) a module loader is added for the inlined bundle, which references a file that is not generated (since it is inlined). This invalid reference causes the import to fail at runtime if the fetch fails before the correct import resolves; it appears to also cause a TypeError ("Cannot read properties of undefined") in tests.

This fix simply excludes external bundles in getLoaderRuntime that have a bundleBehavior of inline.

💻 Examples

From the unit test code:

asyncTransitive.js:

export default import("./inline.js");

inline.js:

import otherHTML from "bundle-text:./inline.html"
export default otherHTML;

inline.html:

<p>test</p>
<script>console.log('hi')</script>

asyncTransitive.js is used as an entry point; generates 2 invalid references:

// ...
module.exports = Promise.all([
    require("./helpers/browser/js-loader")(require("./helpers/bundle-url").getBundleURL("gfgQn") + "inline.4243f809.js" + "?" + Date.now()).catch((err)=>{ /* ... */ }),
    require("./helpers/browser/html-loader")(require("./helpers/bundle-url").getBundleURL("gfgQn") + "inline.0dc63d1b.html" + "?" + Date.now()).catch((err)=>{ /* ... */ }}),
    require("./helpers/browser/js-loader")(require("./helpers/bundle-url").getBundleURL("gfgQn") + "inline.03c983ec.js" + "?" + Date.now()).catch((err)=>{ /* ... */ })
]).then(()=>module.bundle.root("5L8Sl"));
// ...

(inline.4243f809.js and inline.0dc63d1b.html don't exist)

🚨 Test instructions

N/A (unit test added)

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

When dynamically importing a module that itself has an inlined import
(e.g. with bundle-text) a module loader is added for the inlined bundle,
which references a file that is not generated (since it is inlined).
This invalid reference causes the import to fail at runtime if the fetch
fails before the correct import resolves; it appears to also cause a
TypeError ("Cannot read properties of undefined") in tests.

This fix simply excludes external bundles in getLoaderRuntime that have
a bundleBehavior of inline.
@oe
Copy link

oe commented Dec 9, 2022

Confirmed issue! Nice mr!

This mr can fix error like this(caused by invalid references)
CleanShot 2022-12-09 at 22 49 24

but still not working when inlined module is a web worker, here is a demo https://stackblitz.com/edit/github-fshd7k-xtd4gl?file=src/app.js can reproduce this issue:

  1. download project (stackblitz not working online due to lack of supporting of native node modules)
    CleanShot 2022-12-09 at 23 09 21

  2. install and run project yarn && yarn dev

  3. open browser and open devtools, check logs in console

@comp500
Copy link
Author

comp500 commented Apr 9, 2023

I can't seem to reproduce that issue with my change, I just get the compiled bundle of worker.js in the log:
image

The worker itself doesn't run in development because Parcel doesn't know that the module is intended to be a web worker (as you're not using the intended syntax) so it emits the HMR runtime code, which isn't compatible with web workers. Running in production with the files produced from parcel build works fine.

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.

None yet

2 participants