vitejs / vite

Next generation frontend tooling. It's fast!
http://vite.dev
MIT License
68.73k stars 6.21k forks source link

HMR on service worker #5396

Open userquin opened 3 years ago

userquin commented 3 years ago

Describe the bug

I'm trying to add HMR to a service worker, I don't know if I can do that, right now it is failing.

The sw code:

The sw code

The client error:

The client error

The failing code:

The failing code

Reproduction

Here the branch on my repo: https://github.com/userquin/vite-plugin-pwa/tree/feat/add-development-support

pnpm installed globally required and node 12+, I'm testing with node 15 via nvm: once cloned and checkout feat/add-development-support execute pnpm install && pnpm run build from root folder.

To run the sw example on dev, execute, also from root folder, pnpm run example:dev:sw.

Open the app in Chrome, it is registered with type: 'module'.

If you remove the import.meta.hot from examples/vue-basic-inject-manifest/src/sw.ts, the service worker will work.

The logic for the plugin can be found on src/index.ts, search last plugin (name: 'vite-plugin-pwa:dev-sw'), the internal logic on src/dev.ts: the example on examples/vue-basic-inject-manifest.

System Info

System:
    OS: Windows 10 10.0.19042
    CPU: (16) x64 Intel(R) Core(TM) i9-9900K CPU @ 3.60GHz
    Memory: 49.51 GB / 63.95 GB
  Binaries:
    Node: 15.11.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.10 - C:\Program Files\nodejs\yarn.CMD
    npm: 7.6.0 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Chrome: 95.0.4638.54
    Edge: Spartan (44.19041.1266.0), Chromium (94.0.992.50)
    Internet Explorer: 11.0.19041.1202

Used Package Manager

pnpm

Logs

Since it is on client, here the logs on dev tools (see screenshots):

Uncaught ReferenceError: HTMLElement is not defined
    at overlay.ts:118

Validations

userquin commented 3 years ago

I fix packages/vite/src/client/overlay.ts and packages/vite/src/client/client.ts checking for typeof window, but it seems we cannot use dynamic imports on service workers (I think on web workers should work):

imagen

userquin commented 3 years ago

The link if someone interested: https://github.com/w3c/ServiceWorker/issues/1356

userquin commented 3 years ago

A question about building viteon local via pnpm run build-vite script: why each time I build vite, this file packages/vite/LICENSE.md is changed? All licenses removed:

imagen

gotjoshua commented 2 years ago

https://github.com/lencx/vite-plugin-rsw/issues/25#issuecomment-1049274571

bluwy commented 2 years ago

Related: https://github.com/vitejs/vite/issues/7163 and https://github.com/vitejs/vite/issues/2248

kleisauke commented 2 years ago

PR https://github.com/vitejs/vite/pull/9064 fixed for me the HTMLElement is not defined error with HMR on web workers. Perhaps that would also fix the issue on service workers?

deathemperor commented 11 months ago

PR #9064 fixed for me the HTMLElement is not defined error with HMR on web workers. Perhaps that would also fix the issue on service workers?

The issue is now with react-refresh (I think?)

@react-refresh:367 Uncaught ReferenceError: window is not defined
    at @react-refresh:367:1

which is this line

image
alexturpin commented 7 months ago

I am now hitting the issue with $RefreshReg$ as well, after updating Vite to fix the one with overlay.ts

trans-utils.tsx:5 Uncaught ReferenceError: $RefreshReg$ is not defined

My stack trace is on a random (the first in the dependency graph maybe?) bit of JSX.

Has anyone come up with a solution?

cpojer commented 5 months ago

The way to solve this is by adding a side-effectful module that you import before any other modules in your worker.

See initializeWorker.tsx and the example usage in a worker.

ashleyww93 commented 2 weeks ago

PR #9064 fixed for me the HTMLElement is not defined error with HMR on web workers. Perhaps that would also fix the issue on service workers?

The issue is now with react-refresh (I think?)

@react-refresh:367 Uncaught ReferenceError: window is not defined
    at @react-refresh:367:1

which is this line

image

I am still having this issue, after trying to migrate from rewired version of react create app.

The fix by @cpojer alone did not work, I also had to disable the react plugin... which makes vite painful to use while developing a large project.

How can Vite be so popular without having this right?!

It seems to be due to random imports (which do not use jsx) which then, for whatever reason, bundles react-refresh with them.

No HMR is sort of a dealbreaker on a large project like ours.

addy commented 3 days ago

The issue is now with react-refresh (I think?)

@react-refresh:367 Uncaught ReferenceError: window is not defined
    at @react-refresh:367:1

@ashleyww93 Running into the same issue with a new SharedWorker I'm working on. Was worried it was due to some transitive import, which sounds likely. Also wondering if rolling back Vite a few versions would have any effect.

ashleyww93 commented 3 days ago

The issue is now with react-refresh (I think?)

@react-refresh:367 Uncaught ReferenceError: window is not defined
    at @react-refresh:367:1

@ashleyww93 Running into the same issue with a new SharedWorker I'm working on. Was worried it was due to some transitive import, which sounds likely. Also wondering if rolling back Vite a few versions would have any effect.

We couldn’t find the import that causes this for us.

We are importing this as a side effect and it’s now working well.

https://gist.github.com/ashleyww93/12f8350ecc552c1d29ce93271147f325

addy commented 2 days ago

We couldn’t find the import that causes this for us.

We are importing this as a side effect and it’s now working well.

https://gist.github.com/ashleyww93/12f8350ecc552c1d29ce93271147f325

Interesting, it's defined now, but thinks it's being loaded twice due to the presence of $RefreshReg$

if (window.$RefreshReg$) {
  throw new Error(
    "React refresh runtime was loaded twice. Maybe you forgot the base path?"
  );
}

Assuming you aren't hitting that issue? I'm going to just try setting it to undefined or something falsey.