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

HMR on service worker #5396

Open
7 tasks done
userquin opened this issue Oct 23, 2021 · 13 comments · May be fixed by #18554
Open
7 tasks done

HMR on service worker #5396

userquin opened this issue Oct 23, 2021 · 13 comments · May be fixed by #18554
Labels
enhancement New feature or request feat: hmr feat: web workers p2-to-be-discussed Enhancement under consideration (priority)

Comments

@userquin
Copy link
Contributor

userquin commented Oct 23, 2021

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
Copy link
Contributor Author

userquin commented Oct 23, 2021

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
Copy link
Contributor Author

The link if someone interested: w3c/ServiceWorker#1356

@userquin

This comment has been minimized.

@gotjoshua
Copy link

@patak-dev patak-dev added enhancement New feature or request p2-to-be-discussed Enhancement under consideration (priority) and removed pending triage discussion labels May 2, 2022
@bluwy
Copy link
Member

bluwy commented Jun 26, 2022

Related: #7163 and #2248

@kleisauke
Copy link

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?

@deathemperor
Copy link

deathemperor commented Dec 15, 2023

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
Copy link

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
Copy link
Contributor

cpojer commented May 27, 2024

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
Copy link

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.

@userquin userquin linked a pull request Nov 1, 2024 that will close this issue
@addy
Copy link

addy commented Nov 18, 2024

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
Copy link

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
Copy link

addy commented Nov 19, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feat: hmr feat: web workers p2-to-be-discussed Enhancement under consideration (priority)
Projects
None yet