fix: allow dynamically import atmosphere.js for SW context #2901
Conversation
krissvaa
force-pushed
the
fix/sw-context-access
branch
from
f5f6170
to
cf3a028
Compare
krissvaa
force-pushed
the
fix/sw-context-dynamic-import
branch
from
5c613c4
to
33b8cc7
Compare
Env should be optional for other frameworks Fixes #2867
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## fix/sw-context-access #2901 +/- ##
=========================================================
- Coverage 92.54% 92.52% -0.03%
=========================================================
Files 83 83
Lines 2832 2836 +4
Branches 732 733 +1
=========================================================
+ Hits 2621 2624 +3
- Misses 156 157 +1
Partials 55 55
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -0,0 +1,10 @@ | |||
| // / <reference types="vite/client" /> | |||
There was a problem hiding this comment.
| // / <reference types="vite/client" /> | |
| /// <reference types="vite/client" /> |
| (async () => { | ||
| if (!import.meta.env?.VITE_SW_CONTEXT) { | ||
| atmosphere = await import('atmosphere.js').then((module) => module.default); | ||
| } | ||
| })().catch((e) => { | ||
| console.error('Failed to load atmosphere.js', e); | ||
| }); |
There was a problem hiding this comment.
| (async () => { | |
| if (!import.meta.env?.VITE_SW_CONTEXT) { | |
| atmosphere = await import('atmosphere.js').then((module) => module.default); | |
| } | |
| })().catch((e) => { | |
| console.error('Failed to load atmosphere.js', e); | |
| }); | |
| if (!import.meta.env?.VITE_SW_CONTEXT) { | |
| try { | |
| atmosphere = await import('atmosphere.js').then((module) => module.default); | |
| } catch (e: unknown) { | |
| console.error('Failed to load atmosphere.js', e); | |
| } | |
| } |
I'd suggest using top-level await here. Is it possible?
There was a problem hiding this comment.
Also, we can probably check the presence of the document in globalThis here, instead of VITE_SW_CONTEXT.
There was a problem hiding this comment.
top-level await is not allowed according to our project configuration.
Top-level await is not available in the configured target environment ("chrome87", "edge88", "es2020", "firefox78", "safari14" + 2 overrides)
There was a problem hiding this comment.
Let's avoid top-level await for now.
There was a problem hiding this comment.
But that creates a possible race condition, and in some cases the FluxConnection can possibly be created before the Atmosphere is loaded.
Fixes #2867