Skip to content

PWA endpoint errors while called from SW #2867

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

Closed
krissvaa opened this issue Oct 24, 2024 · 2 comments · Fixed by #2900 or #2901 · May be fixed by vaadin/flow-hilla-common#16 or vaadin/flow#20470
Closed

PWA endpoint errors while called from SW #2867

krissvaa opened this issue Oct 24, 2024 · 2 comments · Fixed by #2900 or #2901 · May be fixed by vaadin/flow-hilla-common#16 or vaadin/flow#20470

Comments

@krissvaa
Copy link
Contributor

Describe the bug

Some issues stop endpoints from being called from the Service Worker.
Most of the issues are that window or document references are not available in the worker context.
Which should be quite easy to solve:

Then there are related to the same issue but more complicated issues:

  1. __REGISTER__() includes references to window - but is defined in a different place
    https://github.com/vaadin/hilla/blob/c7d4707995f2235574f48d84a8619ab038beb503/packages/ts/frontend/src/index.ts#L8C1-L8C16

  2. ConnectionState and ConnectionIndicator from @vaadin/common-frontend also have references to window and need to be either fixed in their repo or clone+fixed in hilla separately

    import { ConnectionIndicator, ConnectionState } from '@vaadin/common-frontend';

  3. Lastly there is an issue with atmosphere.js and default exports to Vite. Atmosphere dependency could be excluded by SW Vite plugin or by finding a better fix for exclusion as I am not sure about this approach.

RollupError: node_modules/@vaadin/hilla-frontend/FluxConnection.js (1:7): "default" is not exported by "node_modules/atmosphere.js/lib/browser.js", imported by "node_modules/@vaadin/hilla-frontend/FluxConnection.js".
at getRollupError (file:///app/node_modules/rollup/dist/es/shared/parseAst.js:396:41)
at error (file:///app/node_modules/rollup/dist/es/shared/parseAst.js:392:42)
at Module.error (file:///app/node_modules/rollup/dist/es/shared/node-entry.js:13858:16)
at Module.traceVariable (file:///app/node_modules/rollup/dist/es/shared/node-entry.js:14306:29)
at ModuleScope.findVariable (file:///app/node_modules/rollup/dist/es/shared/node-entry.js:11984:39)
at ChildScope.findVariable (file:///app/node_modules/rollup/dist/es/shared/node-entry.js:7432:38)
at ClassBodyScope.findVariable (file:///app/node_modules/rollup/dist/es/shared/node-entry.js:7432:38)
at ChildScope.findVariable (file:///app/node_modules/rollup/dist/es/shared/node-entry.js:7432:38)
at ChildScope.findVariable (file:///app/node_modules/rollup/dist/es/shared/node-entry.js:7432:38)
at FunctionScope.findVariable (file:///app/node_modules/rollup/dist/es/shared/node-entry.js:7432:38)

Missing CSRF token in calling endpoints is separated in an another issue: #2791

Expected-behavior

All DOM window or document references should be prefixed with self.window && window... or self.document && document... (or some alternative) so that SW could run without encountering them.

Reproduction

sw-app.zip

System Info

Hilla 24.5

@krissvaa krissvaa added bug Something isn't working hilla Issues related to Hilla labels Oct 24, 2024
@krissvaa krissvaa self-assigned this Oct 28, 2024
krissvaa added a commit to vaadin/flow-hilla-common that referenced this issue Nov 11, 2024
krissvaa added a commit to vaadin/flow-hilla-common that referenced this issue Nov 11, 2024
krissvaa added a commit that referenced this issue Nov 12, 2024
krissvaa added a commit that referenced this issue Nov 13, 2024
krissvaa added a commit that referenced this issue Nov 13, 2024
krissvaa added a commit that referenced this issue Nov 13, 2024
krissvaa added a commit that referenced this issue Nov 13, 2024
krissvaa added a commit that referenced this issue Nov 13, 2024
krissvaa added a commit that referenced this issue Nov 13, 2024
krissvaa added a commit that referenced this issue Nov 13, 2024
Env should be optional for other frameworks
Fixes #2867
krissvaa added a commit that referenced this issue Nov 13, 2024
krissvaa added a commit to vaadin/flow that referenced this issue Nov 14, 2024
krissvaa added a commit that referenced this issue Nov 15, 2024
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Hilla 24.6.0.beta2 and is also targeting the upcoming stable 24.6.0 version.

@Lodin Lodin assigned Lodin and krissvaa and unassigned krissvaa and Lodin Nov 27, 2024
platosha added a commit that referenced this issue Apr 14, 2025
* Change window and document DOM access to self checks for SW context
Fixes #2867

* Allow dynamically import atmosphere.js for SW context
Fixes #2867

* Fix dynamic import of atmosphere
Fixes #2867

* Fix dynamic import of atmosphere
Fixes #2867

* Fix dynamic import of atmosphere
Fixes #2867

* Fix dynamic import of atmosphere as top level await not allowed
Env should be optional for other frameworks
Fixes #2867

* refactor(frontend): use top-level import of Atmosphere

* refactor(frontend): collect messages when atmosphere isn't loaded

* chore: unpin and update rollup

* build: re-enable Flow updates for dependencies in workspace

* chore: update depdenencies

---------

Co-authored-by: Vlad Rindevich <[email protected]>
Co-authored-by: Anton Platonov <[email protected]>
Co-authored-by: Luciano Vernaschi <[email protected]>
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Hilla 24.8.0.alpha5 and is also targeting the upcoming stable 24.8.0 version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment