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

fix: check a constructor exists before instantiation #13025

Closed
wants to merge 2 commits into from

Conversation

dsgallups
Copy link

Attempts to fix #13015

Summary of issue:
When using vite-plugin-wasm with vite-plugin-top-level-await, an external async IIFE is passed alongside the generated component constructor output. Until this top level await function completes, the constructor is undefined. Svelte 5's compiler changes assume, for root.svelte, that if constructor[1] is not undefined, constructor[0] is not undefined. This is true in almost all cases, excepting where vite-plugin-top-level-await is used.

It seems like Svelte 4's compiler had a fallback constructor when the passed constructor was undefined. Please let me know if the solution is better situated within the Svelte 5 compiler, or perhaps belongs elsewhere! Thanks for your consideration!


Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.
    • I would be happy to create an e2e test for this using vite-plugin-top-level-await in /tests/apps

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

Edits

  • Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.

Copy link

changeset-bot bot commented Nov 19, 2024

🦋 Changeset detected

Latest commit: 475f4c0

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@sveltejs/kit Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Rich-Harris
Copy link
Member

preview: https://svelte-dev-git-preview-kit-13025-svelte.vercel.app/

this is an automated message

@benmccann
Copy link
Member

Hmm. This feels a bit like changing SvelteKit to fix an issue in a Vite plugin. I'm pretty uncomfortable with this. I'll take the discussion to the issue since there are more details there

@dsgallups
Copy link
Author

Thought so too, seems like a pretty invasive change. Thanks for your advice in the issue!

@dsgallups dsgallups closed this Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Components that require an external top level await during compilation fail in svelte 5
3 participants