Update Browser Extension's Vite Config

[noi5e]

Possible prerequisite #2510 ⬅️ Look at that PR first, particularly this and this comment

Adjusts the vite.config.js to clean up the file/folder structure.

Before

After

Next Steps

Not just aesthetic, this PR paves the way for the following changes:

1. Automatic hotloading of browser extension on changes, for faster development
2. Ability to build the extension with a manifest.json v3.0, a prerequisite to getting the extension published in Chrome & Edge stores
3. Renaming of browser extension files from .js to .ts (see comment )

---

Developer Tools Web Extension Planning Issue: #2127

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
lexical	✅ Ready (Inspect)	Visit Preview	Jul 5, 2022 at 8:29PM (UTC)
lexical-playground	✅ Ready (Inspect)	Visit Preview	Jul 5, 2022 at 8:29PM (UTC)

[noi5e]

Here's a weird warning that npm run build throws: Generated an empty chunk: "popup"

Because of entryFileNames here:

rollupOptions: {
      input: {
        background: resolve(root, 'background', 'index.js'),
        content: resolve(root, 'content', 'index.js'),
        devtools: resolve(root, 'devtools', 'index.html'),
        panel: resolve(root, 'panel', 'index.html'),
        popup: resolve(root, 'popup', 'index.html')
      },
      output: {
        entryFileNames: (chunk) => `src/${chunk.name}/index.js`
      }
    }

Unlike the other folders/chunks, popup does not contain a .js file, only index.html:

I've been looking into the Vite/rollup docs but still lost looking for alternatives or customization options... So any leads would be appreciated 🙏

[thegreatercurve]

this

Here's a weird warning that npm run build throws: Generated an empty chunk: "popup"

Because of entryFileNames here:

rollupOptions: {
      input: {
        background: resolve(root, 'background', 'index.js'),
        content: resolve(root, 'content', 'index.js'),
        devtools: resolve(root, 'devtools', 'index.html'),
        panel: resolve(root, 'panel', 'index.html'),
        popup: resolve(root, 'popup', 'index.html')
      },
      output: {
        entryFileNames: (chunk) => `src/${chunk.name}/index.js`
      }
    }

Unlike the other folders/chunks, popup does not contain a .js file, only index.html:

I've been looking into the Vite/rollup docs but still lost looking for alternatives or customization options... So any leads would be appreciated 🙏

@noi5e Does the whole app it still build correctly if you omit it from the inputs?

[thegreatercurve]

Updating this to unblock you as I'll be away for a few days. You can't merge it anyway without fixing the existing TS errors. The other files which are JS should ideally be TS as well, if this config allows that to work correctly

[noi5e]

The other files which are JS should ideally be TS as well, if this config allows that to work correctly

Just pushed new commits, every .js file is now .ts.

The build still works, manually tested in Chrome/Firefox/Edge.

I ran into some troubles with absolute/relative path stuff, like in this example:

chrome.devtools.panels.create(
  'Lexical',
  '/../../favicon-32x32.png',
  '/src/panel/index.html',
);

If the path is ./index.html, the panel builds in Firefox, but not in Chrome/Edge.

If the path is src/panel/index.html, the panel builds in Chrome/Edge, but not in Firefox.

Also, if I put favicon-32x32.png in an images folder and have the path be relative ../../images/favicon-32x32.png, for some reason Vite doesn't parse the URL. The icon has to be in the public folder.

Otherwise, this PR seems to work fine. Just have to take care of things in #2510 first, then this should be merge ready.

[noi5e]

@noi5e Does the whole app it still build correctly if you omit it from the inputs?

If I omit popup: resolve(root, 'popup', 'index.html') from vite.config.js, the popup folder doesn't get included in the build at all!

[noi5e]

@acywatson had a suggestion here:

can these be constants somewhere in a different shared module, maybe?

I thought it made sense to try an implementation of that here in this PR, which I made in the latest commit add ES6-style import in browser extension.

The execution is very awkward because browser extension content and background scripts do not allow ES6 import without workarounds.

Currently the implementation does not work because I'm encountering a block. In src/content/index.ts, I use dynamic import to load the content script:

const src = chrome.runtime.getURL('./content.ts');

However vite.config.js doesn't seem to be able to parse this URL. content.ts is not included in the final build.

So I guess the question is if it's going to be worth it right now to pursue ES6 style imports and modules in the content and background scripts... And if so, any suggestions for how to update the Vite config to build this properly?

[acywatson]

@acywatson had a suggestion here:

can these be constants somewhere in a different shared module, maybe?

I thought it made sense to try an implementation of that here in this PR, which I made in the latest commit add ES6-style import in browser extension.

The execution is very awkward because browser extension content and background scripts do not allow ES6 import without workarounds.

Currently the implementation does not work because I'm encountering a block. In src/content/index.ts, I use dynamic import to load the content script:

const src = chrome.runtime.getURL('./content.ts');

However vite.config.js doesn't seem to be able to parse this URL. content.ts is not included in the final build.

So I guess the question is if it's going to be worth it right now to pursue ES6 style imports and modules in the content and background scripts... And if so, any suggestions for how to update the Vite config to build this properly?

Yea, let's not worry about it for now. Thanks for looking into it!

[noi5e]

Rebased off main after #2510 was merged, and undid merge conflicts.

This seems ready to merge, the only thing stopping it is unrelated CI failures.

One important thing to note is that I had to do a lot of TS linter disabling in background.js to get this to pass. This was based off @thegreatercurve's suggestion here:

"cast" the types to any to remove the type checks, disable the lint warning, and one of us can then do a fix PR on top to add the types.

(I'm also currently studying up on TypeScript to see if I can give this a shot myself)