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

Content Script ESM Support #357

Open
3 of 4 tasks
aklinker1 opened this issue Jan 18, 2024 · 32 comments
Open
3 of 4 tasks

Content Script ESM Support #357

aklinker1 opened this issue Jan 18, 2024 · 32 comments
Labels

Comments

@aklinker1
Copy link
Collaborator

aklinker1 commented Jan 18, 2024

Feature Request

As discussed in #335, it is possible to load ESM content scripts using a dynamic import. The downside is that since it's async, the standard run_at option has basically no effect.

I propose adding a new option to defineContentScript: type: "module". Similar to the background's type: "module" option.

When WXT sees an async content script, it will load the script asynchronously using a dynamic import.

await import(
  /* @vite-ignore */
  browser.runtime.getURL("path/to/chunk/entrypoint.js")
);

Questions:

  • Does the run_at in the manifest make a difference in loading speed when using a dynamic import

    Jan 20, 2024: Slightly, we should respect it, and allow configuring it, with the knowledge that document_start doesn't work. See Content Script ESM Support #357 (comment)

  • Can async content scripts be bundled in one step alongside HTML entrypoints, or do they have to be separated into their own step? My concern here is mixing chunks with side-effects that only work in HTML pages

    Jan 20, 2024: I don't think this is an issue, it's only an issue for the background service worker where there's no window global

  • How will dev mode function and reload content scripts. What about main world content scripts, should those be grouped with the regular HTML file builds, or be in their own build step?

    Feb 2, 2024: Have not tested dev mode yet, but I am building the ESM background script with the other extension pages, so I'll do the same with isolated world content scripts. Main world content scripts will be combined with the sandbox pages since those don't have access to the browser APIs, like sandbox pages.

  • Should type: "module" be the default value? It works well with the default run_at: "document_idle", and will likely provide a much better dev experience.

    Jan 21, 2024: If dev mode works well, yes, it will be the default.

    Feb 2, 2024: Initially, I'm going to make this an opt-in just like the background's type: "module". If it's working well for people, I'll try and make the the default before v1.0.

Is your feature request related to a bug?

#335

What are the alternatives?

No real alternatives to the feature as a whole. Instead of adding a new field, we could use runAt: "async", but that wouldn't provide a way to set the actual run_at in the manifest. That said, the run_at doesn't really matter, it can cause the browser to import the script earlier, but the code will never run before the DOMContentLoaded event.

Additional context

CC @yunsii

This will fix: #270

@aklinker1 aklinker1 added this to the v1.0 milestone Jan 18, 2024
@yunsii
Copy link
Contributor

yunsii commented Jan 19, 2024

This feature is really nice, I am exploring how to implement HMR in content script like CRXJS, it is really a magic.

@aklinker1
Copy link
Collaborator Author

aklinker1 commented Jan 19, 2024

I've switch my proposal to use type: "module" instead of runAt: "async".


@yunsii I'd like to structure the outputs like this:

.output/
  <target>/
    chunks/
      ...
    content-scripts/
      <name>.js
      <name>-loader.js

We may need to include the hash in dev mode to the non-loader file for HMR to work, I'm not sure.

At a minimum, the loader would look something like this:

import(
  /* @vite-ignore */
  browser.runtime.getURL("/content-scripts/<name>.js")
);

I'm also not sure where we need to call the main function of the content script. Inside the <name>.js like the above loader would do, or inside the <name>-loader.js, like this:

const { default } = await import(
  /* @vite-ignore */
  browser.runtime.getURL("/content-scripts/<name>.js")
);
default.main(...);

If I remember correctly, dynamically imported modules don't have access to the chrome or browser global, so we may need to use the second example, and actually run the main function of the content script inside the loader, which it has access to those globals.

@aklinker1 aklinker1 changed the title "async" content script support Content Script ESM Support Jan 19, 2024
@yunsii
Copy link
Contributor

yunsii commented Jan 20, 2024

@aklinker1 Both examples seems ok, here is my demo https://github.com/yunsii/chrome-extension-raw-demo/blob/master/src/js/isolated_content_script.js
image

@aklinker1
Copy link
Collaborator Author

Cool, thanks for researching this! If both work, we'll go with whatever option makes the most sense during the implementation.

@yunsii
Copy link
Contributor

yunsii commented Jan 20, 2024

we'll go with whatever option makes the most sense during the implementation.

What's the meaning? I do not understand exactly.

@aklinker1
Copy link
Collaborator Author

aklinker1 commented Jan 20, 2024

Did some testing with the run_at field. Behaved exactly as I expected. It could be useful to set the run_at field. For ecxample, with document_start you load your script a little bit earlier, but the code will never run before the DOMContentLoaded event, unlike without a dynamic import.

esm-run-at-testing.zip

Screenshot 2024-01-19 at 11 40 07 PM

@aklinker1
Copy link
Collaborator Author

aklinker1 commented Jan 20, 2024

we'll go with whatever option makes the most sense during the implementation.

What's the meaning? I do not understand exactly.

@yunsii Between the two options (running main inside the imported file, or running main inside the loader), since both work, we can use either one in WXT. So whichever option is easier to do while implementing this feature, we'll go with.

@aklinker1
Copy link
Collaborator Author

Also, for future reference, here are the minimum requirements to get ESM content scripts working:

  1. Import statements must include the file extension
    This does not work:
    - import { logId } from './utils/log';
    This works:
    + import { logId } from './utils/log.js';
  2. All imported files (chunks and non-loader entrypoints) must be listed in web_accessible_resources
     {
       "matches": [...],
       "resources": ["content-scripts/<name>.js", "chunks/*"]
     }

Here's a minimal example with an ESM service worker and content script sharing the ES module utility.

minimal-esm-chrome-extension.zip

@yunsii
Copy link
Contributor

yunsii commented Jan 20, 2024

So it means that document_start and document_idle can use type: "module"?

@aklinker1
Copy link
Collaborator Author

So it means that document_start and document_idle can use type: "module"?

You can use all three run_at values (document_start, document_end, and document_idle) with type: "module". But only document_idle will behave the same way with or without type: "module"

@yunsii
Copy link
Contributor

yunsii commented Jan 20, 2024

Did some testing with the run_at field. Behaved exactly as I expected. It could be useful to set the run_at field. For ecxample, with document_start you load your script a little bit earlier, but the code will never run before the DOMContentLoaded event, unlike without a dynamic import.

esm-run-at-testing.zip

Screenshot 2024-01-19 at 11 40 07 PM

I see, with type: "module", this means all run_at values can work, but dynamic imports with document_start and document_end have to wait until the proper timing to run?

@aklinker1
Copy link
Collaborator Author

Yup, exactly!

@aklinker1
Copy link
Collaborator Author

aklinker1 commented Jan 20, 2024

I made a quick Vite project to spike out what's required to build an ESM chrome extension. Here it is:

minimal-vite-esm-extension.zip

Really all you have to do is add the loader to the bundle in a custom plugin during the generateBundle step.

// vite.config.ts
import { Plugin } from "vite";
import { defineConfig } from "vite";

const esmContentScriptLoader = (): Plugin => ({
  name: "esm-content-script-loader",
  generateBundle(_options, bundle, _isWrite) {
    // Add the loader to the bundle before the bundle is written to the disk
    bundle["content-script-loader.js"] = {
      type: "asset",
      fileName: "content-script-loader.js",
      name: "content-script-loader",
      needsCodeReference: false,
      source: `(async () => {
  console.log("Importing 'content-script'...")
  await import(
    /* vite-ignore */
    chrome.runtime.getURL('/content-script.js')
  )
  console.log("Imported 'content-script'!")
})()
`,
    };
  },
});

export default defineConfig({
  build: {
    rollupOptions: {
      input: {
        popup: "src/popup.html",
        "content-script": "src/content-script.ts",
        background: "src/background.ts",
      },
      output: {
        format: "esm",
        entryFileNames: "[name].js",
      },
    },
    // Not necessary, just for clearity when looking at output files
    minify: false,
  },
  plugins: [esmContentScriptLoader()],
});

Otherwise vite pretty much builds everything else correctly.

This doesn't include a working dev mode, just the build. There's a lot of complex pre-rendering that needs to happen for dev mode to work, and that's all setup in WXT, so it makes sense to implement it in WXT, then test dev mode.

@aklinker1
Copy link
Collaborator Author

I've prioritized #57 over this issue, so I still haven't done any additional work on this yet.

@yunsii
Copy link
Contributor

yunsii commented Feb 28, 2024

image

It may have to be improved priority? These erros occured after I upload our extension to firefox extension workshop.

@aklinker1
Copy link
Collaborator Author

I haven't had any more time to spend on this the last 3 weeks, I've been tackling the smaller bugs people have reported recently. But don't worry, this is at the top of my priorities when I have a free weekend to focus on it.

@aklinker1
Copy link
Collaborator Author

aklinker1 commented Apr 16, 2024

Update

I tried setting up dev mode with the dynamic import loaders, but ran into a problem: CSS from the page is always applied to the page, and there's no way to change that. So basically, createShadowRootUi won't work in ESM content scripts...

Other than that, I'm gonna keep going forward, and maybe I'll find a workaround, but just wanted to leave an update here. I haven't attempted to add HMR yet, but have a good idea about how I'd go about it.

@yunsii
Copy link
Contributor

yunsii commented Apr 17, 2024

CSS from the page is always applied to the page

What's the meaning? Shadow dom CSS from createShadowRootUi will apply to the page?

@aklinker1
Copy link
Collaborator Author

The way Vite deals with CSS in dev mode, it reads and transforms a file, then either adds or removes a style block for each module to the document's head element. When a module is saved, it deletes that module's style block and adds a new one with the changes.

WXT, on the other hand, does a full build for each content script so we have a CSS file that can be loaded into the extension. However, moving to esm, there is no CSS file exists and we have to rely on vite's method of adding style blocks to the page as modules change. Shadow roots, however, need the style injected inside the shadow root.

Vite doesn't provide a way to change where the esm style blocks get inserted. They're always inserted in the document's head block. I haven't seen away at least to change that. Ideally, the simple solution would be to just tell Vite to insert the style into the shadow root instead of the document's head. But like I said, I don't know how to do that or if that's even possible.

@aklinker1
Copy link
Collaborator Author

https://github.com/vitejs/vite/blob/main/packages%2Fvite%2Fsrc%2Fclient%2Fclient.ts#L418

Here's where Vite appends the style to document.head. maybe we could override the function on the element? Never tried that before.

@yunsii
Copy link
Contributor

yunsii commented Apr 24, 2024

Is there any approach to override the function?

@aklinker1 aklinker1 moved this to Todo in Aaron's Work Jul 7, 2024
@aklinker1 aklinker1 removed this from the v1.0 milestone Jul 27, 2024
@aklinker1 aklinker1 added this to the v1.1 milestone Jul 27, 2024
@aklinker1
Copy link
Collaborator Author

aklinker1 commented Aug 15, 2024

I'm going to close this issue as not-planned. I haven't figured out how to override this function without breaking something else, so I don't want to support them right now.

This has been sitting in the back of my mind for 4 months now, and I don't want to keep working on it. If someone else wants to give it a go, please do!

@aklinker1 aklinker1 closed this as not planned Won't fix, can't repro, duplicate, stale Aug 15, 2024
@github-project-automation github-project-automation bot moved this from Todo to Done in Aaron's Work Aug 15, 2024
@yunsii
Copy link
Contributor

yunsii commented Aug 16, 2024

I think the way like eslint-ts-patch to patch vite is a good choice, wxt has taken over vite after all.

@aklinker1
Copy link
Collaborator Author

he way like eslint-ts-patch to patch vite is a good choice, wxt has taken over vite after all.

Are you talking about how you install the patch "as" eslint? Like this:

npm i -D eslint-ts-patch eslint@npm:eslint-ts-patch

So you're recommending I fork vite and use a custom version? Nah, that doesn't seem worth it to me. I would rather open a PR and slowly work towards adding support for a feature like this, but still, I'm not convinced this is feature is necessary. It's really just a performance improvement during development. I think there are other improvements that can be made before this one.

@yunsii
Copy link
Contributor

yunsii commented Aug 16, 2024

So you're recommending I fork vite and use a custom version?

I don't think so, it seems a special API const Module = require('node:module') can be used to replace specific file with custom rule like: https://github.com/antfu/eslint-ts-patch/blob/50469598f846bb23102b0b7de3405dad53481b1b/lib/register.js#L103-L120

So I think the way can be try. Make a PR to vite is a best solution absolutely, but it must cost so much time to merge.


As for

It's really just a performance improvement during development.

Extension bundle files maybe too large to parse in firefox extension workshop like I said before #357 (comment) So our extension still not support firefox for now 😂

@1natsu172
Copy link
Contributor

FYI, I found a related issue/pr on the vite side. But it seems to be a low priority in the vite team.
vitejs/vite#11855
vitejs/vite#12206

Someone has created a 3rd party plugin, but I don't know if this will work well for our use case.
https://github.com/hood/vite-plugin-shadow-style/tree/main

@aklinker1
Copy link
Collaborator Author

The main problem with all these approaches is it will only work for 1 shadow root UI. If you need multiple, it won't work.

@aklinker1
Copy link
Collaborator Author

Extension bundle files maybe too large to parse in firefox extension workshop like I said before #357 (comment) So our extension still not support firefox for now 😂

@yunsii it's not ideal, but you can implement basic ESM content script support yourself. Here's an example of how to set it up.

https://github.com/wxt-dev/examples/tree/main/examples/esm-content-script-ui

@aklinker1 aklinker1 removed this from the v1.1 milestone Aug 16, 2024
@yunsii
Copy link
Contributor

yunsii commented Aug 17, 2024

It means we can use dynamic import manually anywhere now? the dynamic import modules will not be bundle into the entry file?

@aklinker1
Copy link
Collaborator Author

aklinker1 commented Aug 17, 2024

@yunsii no, WXT will never produce code-split ESM by itself.

This line tells vite to not analyze or bundle the dynamic import, and leave it as-is in the final output.

https://github.com/wxt-dev/examples/blob/main/examples%2Fesm-content-script-ui%2Fentrypoints%2Fcontent%2Findex.ts#L4-L7

Then you are then responsible for making sure the imported file exists, which is done with a custom vite build here:

https://github.com/wxt-dev/examples/blob/main/examples%2Fesm-content-script-ui%2Fmodules%2Fesm-builder.ts#L32

@yunsii
Copy link
Contributor

yunsii commented Aug 18, 2024

I see, with WXT Reusable Modules to custom build esm modules and then import them manually.

But it still not easy to use, why not integrated directly by WXT?

@aklinker1
Copy link
Collaborator Author

Hmm, I got it stuck in my head that this issue was about HMR, but if we ignore that and continue to do separate, code-split enabled builds with reloads like content scripts work today... It might be possible to implement a generic solution. Let me think on this more.

@aklinker1 aklinker1 reopened this Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Todo
Development

Successfully merging a pull request may close this issue.

3 participants