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

feat: allow snippets to be exported from module scripts #14315

Draft
wants to merge 22 commits into
base: main
Choose a base branch
from

Conversation

trueadm
Copy link
Contributor

@trueadm trueadm commented Nov 15, 2024

Closes #10350. Had to add a hacky bit of logic to prevent Acorn from complaining about bad identifiers being in the script exports.

Copy link

changeset-bot bot commented Nov 15, 2024

🦋 Changeset detected

Latest commit: 811843b

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

This PR includes changesets to release 1 package
Name Type
svelte 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-svelte-14315-svelte.vercel.app/

this is an automated message

Copy link
Contributor

Playground

pnpm add https://pkg.pr.new/svelte@14315

@Rich-Harris
Copy link
Member

If we're going to do the Acorn hack (and I can't think of a better alternative; this seems like the smartest approach as long as we're reasonably confident that the API won't change outside a major version change) then we're going to need to throw our own compile errors

@Rich-Harris
Copy link
Member

@JReinhold
Copy link

This would be huge for Storybook's Svelte language integration, where we currently have this ugly API for setting a global snippet as the render target.

This change would allow us to get the Svelte integration closer to the rest of the Storybook API, which would be great for knowledge transferability.

For us, it's really only about having the snippet variable available in the module context, and not about exporting it at all, but I see in the playground output that that is indeed what's happening, so that's great.
I do understand though that 95 % of the use case for this is for exporting purposes. I just don't hope that it only works if you also export the snippet, as that would probably break our use case. (currently in this PR it works even if you don't export it 🎉)

@Conduitry
Copy link
Member

Should we be concerned about what's required in other dev-time tooling to support exporting something that doesn't look like it's declared? Or does other tooling tend to parse this more laxly than what's happening in Acorn?

@Rich-Harris
Copy link
Member

Errr good question. I think it's probably fine, but do you have anything specific in mind?

@Conduitry
Copy link
Member

Not anything in particular, just: is this going to freak out language tools / intellisense? is this going to freak out Prettier? is this going to freak out ESLint?

@ottomated
Copy link
Contributor

Does not work with language tools
image

@dummdidumm
Copy link
Member

yes this needs adjustment in language-tools. will work on this soon.

@Rich-Harris Rich-Harris marked this pull request as draft November 20, 2024 15:54
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.

Svelte 5: Access snippets from context module
6 participants