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

Safari fails to load islands: Unhandled Promise Rejection: ReferenceError: Cannot access uninitialized variable. #10055

Closed
1 task
mb21 opened this issue Feb 8, 2024 · 8 comments · Fixed by #10075
Labels
- P3: minor bug An edge case that only affects very specific usage (priority)

Comments

@mb21
Copy link

mb21 commented Feb 8, 2024

Astro Info

Astro                    v4.3.5
Node                     v18.17.1
System                   macOS (arm64)
Package Manager          npm
Output                   server
Adapter                  @astrojs/node
Integrations             @astrojs/solid-js

If this issue only occurs in one browser, which browser is a problem?

Safari (reproducable in Safari 17.3 on macOS and Safari on iOS 17.3)

Describe the Bug

I have two framework islands on my page (solid.js in this case, but I believe that's irrelevant). One of the islands sometimes fails to load completely. It's not always the same island that fails: sometimes both load, and sometimes one and sometimes the other (also sometimes it starts load, rendering something client-side, but then seems to fail later as the buttons are not working). Interestingly, the error logged to the safari console happens always in the DOM of the first island (even though that island then works perfectly fine, while the other one is broken):

In Safari dev tools it looks as follows (note which function call is underlined):
image

Here the relevant code as text:

                <script>
                (() => {
                    var e = async t => {
                        await (await t())()
                    };
                    (self.Astro || (self.Astro = {})).only = e;
                    window.dispatchEvent(new Event("astro:only"));
                })();

I've only managed to reproduce this in a production build, but even a production build running locally. All network requests completed with a 200.

Seems the code where the error occurs is just above this code. So probably it's an import that's generated by esbuild? That double await seems kinda funky anyway, or is that something you recognize?

I found this interesting comment on the svelte issue tracker:

We finally resolved this issue by removing the esnext build target in our vite config, which was there for a couple of top-level awaits. This bug in webkit is what ultimately made us try this update.

TL;DR:

[in Safari], the following code fails when importing the dependency more than once.

export let bar = 'foo'
await 0

I tried changing astro.config to:

export default defineConfig({
  vite: {
    build: {
      target: 'es2021',
    },
  ...

but when building then I get:

Top-level await is not available in the configured target environment ("es2021" + 2 overrides)

Somebody else has seen this error in their astro project and/or even managed to fix it?

What's the expected result?

All islands hydrate properly.

Link to Minimal Reproducible Example

So far I've not been able to reproduce this with a minimal example. Perhaps it requires a certain amount of code to trigger the race condition?

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Feb 8, 2024
@mb21 mb21 changed the title Safari: Unhandled Promise Rejection: ReferenceError: Cannot access uninitialized variable. Safari fails to load islands: Unhandled Promise Rejection: ReferenceError: Cannot access uninitialized variable. Feb 8, 2024
@mb21
Copy link
Author

mb21 commented Feb 8, 2024

Okay, I actually traced this down to a top-level await in one of the modules (awaiting a dynamic import, nested in a bunch of ifs etc. – that's why I didn't see it for so long). That module was included in both islands and that's why it triggered the Safari bug.

Minimal repro: https://stackblitz.com/edit/github-4s1seb?file=src%2Fcomponents%2FCounter.tsx

So I suppose I'll have to work around that myself. Not sure if there would be a way for Astro to surface this error better... but I believe there are a bunch of tickets for improving error handling of islands already... so feel free to close this issue.

@lilnasy
Copy link
Contributor

lilnasy commented Feb 8, 2024

I remember trying to fix this. I can't recall if it was a reported issue or something I had run into in a project. I didn't get anywhere, I remember that much, so thanks for investigating!

@lilnasy
Copy link
Contributor

lilnasy commented Feb 8, 2024

I can see the error in dev, not in the final build, and the component code runs correctly despite it. Does that sound right?

@lilnasy lilnasy added the needs response Issue needs response from OP label Feb 8, 2024
@mb21
Copy link
Author

mb21 commented Feb 9, 2024

Yes, I think it doesn't depend on dev or prod build. it's just a race condition... but if two modules happen to load the same module (the one with the top-level await), then you run into this.

@lilnasy
Copy link
Contributor

lilnasy commented Feb 9, 2024

I was able to get the error to point to the component that failed to render. No more details, however. Do you think it could be a quirk of safari or solid's compiler that's creating particularly unhelpful errors?

Compiled Counter.tsx
import { createHotContext as __vite__createHotContext } from "/@vite/client";
import.meta.hot = __vite__createHotContext("/src/components/Counter.tsx");
import { template as _$template } from "/node_modules/.vite/deps/solid-js_web.js?v=ac79f996";
import { delegateEvents as _$delegateEvents } from "/node_modules/.vite/deps/solid-js_web.js?v=ac79f996";
import { getNextElement as _$getNextElement } from "/node_modules/.vite/deps/solid-js_web.js?v=ac79f996";
import { runHydrationEvents as _$runHydrationEvents } from "/node_modules/.vite/deps/solid-js_web.js?v=ac79f996";
import { insert as _$insert } from "/node_modules/.vite/deps/solid-js_web.js?v=ac79f996";
import { $$component as _$$component } from "/@solid-refresh";
import { $$refresh as _$$refresh } from "/@solid-refresh";
import { $$registry as _$$registry } from "/@solid-refresh";
const _REGISTRY = _$$registry();
var _tmpl$ = /* @__PURE__ */
    _$template(`<div class=counter><button>-</button><pre></pre><button>+`),
    _tmpl$2 = /* @__PURE__ */
    _$template(`<div class=counter-message>`);
var Counter = _$$component(_REGISTRY, "Counter", function Counter2(props) {
    const [count, setCount] = createSignal(0);
    const add = () => setCount(count() + 1);
    const subtract = () => setCount(count() - 1);
    return [(() => {
        var _el$ = _$getNextElement(_tmpl$),
            _el$2 = _el$.firstChild,
            _el$3 = _el$2.nextSibling,
            _el$4 = _el$3.nextSibling;
        _el$2.$$click = subtract;
        _$insert(_el$3, count);
        _el$4.$$click = add;
        _$runHydrationEvents();
        return _el$;
    })(), (() => {
        var _el$5 = _$getNextElement(_tmpl$2);
        _$insert(_el$5, () => props.children);
        return _el$5;
    })()];
}, {
    location: "src/components/Counter.tsx:6:15"
});
import { createSignal } from "/node_modules/.vite/deps/solid-js.js?v=ac79f996";
import "/src/components/Counter.css";
await 0;
export default Counter;
if (import.meta.hot) {
    _$$refresh("vite", import.meta.hot, _REGISTRY);
    import.meta.hot.accept();
}
_$delegateEvents(["click"]);

@lilnasy lilnasy added - P3: minor bug An edge case that only affects very specific usage (priority) and removed needs response Issue needs response from OP needs triage Issue needs to be triaged labels Feb 9, 2024
@mb21
Copy link
Author

mb21 commented Feb 10, 2024

It's probably Safari... actually the minmimal example (instead of Counter.tsx) is just a framework component that contains:

export let bar = 'foo'
await 0

But yeah, probably there's not much Astro can do, beyond improving the error message to say something like Failed to load framework component Counter.tsx or something like that. Now you just get the error pointing to that weird double await, and unless you know astro's internal you don't really understand what that code is doing there, that it's loading an island...

And probably good that there is this issue now so people find it via google...

@MrHBS
Copy link

MrHBS commented Feb 10, 2024

Is it this Safari bug?

@mb21
Copy link
Author

mb21 commented Feb 12, 2024

@MrHBS yes, that's the one I linked above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P3: minor bug An edge case that only affects very specific usage (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants