crxjs / chrome-extension-tools

Bundling Chrome Extensions can be pretty complex. It doesn't have to be.
https://crxjs.dev/vite-plugin
2.75k stars 182 forks source link

CRXJS breaks dynamic imports by removing "" around imports #608

Closed A-Shleifman closed 1 year ago

A-Shleifman commented 1 year ago

Build tool

Vite

Where do you see the problem?

Describe the bug

CRXJS breaks dynamic imports by removing "" around imports

input
import('./test').then(console.log);
output
import(/src/test.ts__t--1670835051549.js).then(console.log);

Imports should be wrapped in quotes, otherwise, they don't work. These imports work when the project is launched with Vite without CRXJS.

Reproduction

Example provided above

Logs

No response

System Info

System:
    OS: macOS 13.0
    CPU: (10) arm64 Apple M1 Max
    Memory: 12.58 GB / 64.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.17.0 - ~/.nvm/versions/node/v16.17.0/bin/node
    npm: 8.15.0 - ~/.nvm/versions/node/v16.17.0/bin/npm
  Browsers:
    Chrome: 108.0.5359.98
    Safari: 16.1
  npmPackages:
    @crxjs/vite-plugin: ^2.0.0-beta.6 => 2.0.0-beta.7
    vite: ^3.2.4 => 3.2.4

Severity

blocking an upgrade

lionelhorn commented 1 year ago

Same issue with 2.0.0-beta.9

@A-Shleifman Did you find a workaround / know a previous version that worked?

System:
    OS: Windows 10 10.0.19044
    CPU: (12) x64 Intel(R) Core(TM) i7-10750H CPU @ 2.60GHz
    Memory: 38.72 GB / 63.75 GB
  Binaries:
    Node: 16.14.2 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.18 - C:\Program Files\nodejs\yarn.CMD
    npm: 9.1.2 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Spartan (44.19041.1266.0), Chromium (108.0.1462.54)
    Internet Explorer: 11.0.19041.1566
  npmPackages:
    @crxjs/vite-plugin: ^2.0.0-beta.9 => 2.0.0-beta.9
    vite: ^4.0.2 => 4.0.2
A-Shleifman commented 1 year ago
const distPath = '/src/utils/index.ts.js';
await import(distPath);

This goes around Vite (which is bad) and it works, BUT only if this file has already been transpiled before. Any changes to this file and descendants will never be loaded even after a restart. This wouldn't work even as a temporary solution.

lionelhorn commented 1 year ago

@A-Shleifman I was able to find the source of the issue. You can have a look at my fork https://github.com/crxjs/chrome-extension-tools/compare/main...lionelhorn:chrome-extension-tools:86cd570b7a46fcf551df7faf745e7a3a568ddb26

A-Shleifman commented 1 year ago

Thank you, @lionelhorn, for looking into this and finding a way to fix the problem. I dug a bit deeper and apparently it's caused by an inconsistency/bug in the lexer. I opened an issue.

Regarding your fork:

1) It looks like your editor is not respecting the project's prettier config. 2) I saw another PR which was trying to solve the same (posix) issue a while ago. https://github.com/crxjs/chrome-extension-tools/pull/474 3) The line new URL(_id.slice(PREFIX.length)) makes the build fail

Do you mind if we merge the fix for this issue separately from the posix fix?

lionelhorn commented 1 year ago

Thanks @A-Shleifman for digging deeper.

  1. It looks like your editor is not respecting the project's prettier config.

Indeed, realized that prettier issue after the fact. I didn't have time to do a proper PR at the time, that's why I only linked my fork for hints to those interesed.

  1. I saw another PR which was trying to solve the same (posix) issue a while ago. fixed build vite-plugin on windows (fix #473) #474

I've seen that PR. That's what I added dirtily on top of https://github.com/crxjs/chrome-extension-tools/commit/7342ff05c0796431e3f0e919372a7a8a33146550

  1. The line new URL(_id.slice(PREFIX.length)) makes the build fail

Didn't get a chance to test the modifications outside of windows. Build succeeded on windows after the code change but may have broken build for others.

A-Shleifman commented 1 year ago

If it helps, this is the error I got when building your fork on my mac:

[!] (plugin bundleClientCode) TypeError: Could not load client//Users/redacted/chrome-extension-tools/packages/vite-plugin/src/client/es/hmr-client-worker.ts (imported by src/node/plugin-background.ts): Invalid URL
TypeError: Could not load client//Users/redacted/chrome-extension-tools/packages/vite-plugin/src/client/es/hmr-client-worker.ts (imported by src/node/plugin-background.ts): Invalid URL
    at new NodeError (node:internal/errors:387:5)
    at URL.onParseError (node:internal/url:564:9)
    at new URL (node:internal/url:640:5)
    at Object.load (/Users/redacted/chrome-extension-tools/packages/vite-plugin/rollup.config.ts:68:21)
    at /Users/redacted/chrome-extension-tools/node_modules/.pnpm/rollup@2.78.1/node_modules/rollup/dist/shared/rollup.js:22841:40
A-Shleifman commented 1 year ago

@jacksteamdev, can we merge #630?