-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
fix: use fine grained for template if the component is not explicitly in legacy mode #16232
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
Conversation
🦋 Changeset detectedLatest commit: ed16aa2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good apart from one check
packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mhhhm will these tests actually test this correctly? IIRC runtime-runes
forces all components to compile in runes mode. Might need some test setup tweaks
Uhm...yeah I think yes...let me change that |
I tweaked the test suite to give precedence to the defined |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work!
@paoloricciuti does it mean that now I can expect runes mode to be a default until I explicitly enable legacy mode? |
Not exactly...this only really applies to the template, since before #16100 reading state in the template of a legacy component would still track said state. Now if we are not 100% sure the component it's in legacy mode we preserve that behaviour but as soon as you use something that clearly define the mode (like |
Sound great, thank you! |
Closes #16199
Closes #16229
Closes #16194
The ideal would've been to refactor
runes
from a boolean to an enum but I'll be honest I was quite scared of touching it because there are a bunch of places where we use it and we also move it to other variables...basically if we skip one of this it will break stuff.So i went for a
maybe_runes
boolean that checks there's any trace of explicit legacy mode:export let
$$props
or$$restProps
<svelte:options runes={false} />
Before submitting the PR, please make sure you do the following
feat:
,fix:
,chore:
, ordocs:
.packages/svelte/src
, add a changeset (npx changeset
).Tests and linting
pnpm test
and lint the project withpnpm lint