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

perf: use unchanged primitives directly in template attributes #10015

Closed
wants to merge 31 commits into from

Conversation

benmccann
Copy link
Member

@benmccann benmccann commented Dec 27, 2023

There are still additional cases that can be optimized. This PR is already a good size, so I didn't want to be exhaustive. We can always follow up with improvements. This is enough to allow us to optimize @sveltejs/enhanced-img.

We cannot inline variables referred to the in <script> block that invoke any function or constructor during their initialization because we don't know if they may have randomness or side-effects. However, we have no such restriction with <script context="module">, so there's a performance benefit to using that construct. We will need to generate image URLs there in @sveltejs/enhanced-img and users creating such constants manually should prefer to do so there as well.

I had to update a test. It was asserting that we removed a space, but that's not really the important part of the test and this behavior of keeping the space seems better. What it was really testing was that "123" was not changed to "13" during hydration per #6832.

I had to add back the GlobalBindings constant. I really couldn't think of any other way to avoid conflicting variable names at the top level. The existing solutions for avoiding global conflicts don't work for hoisting.

This PR is based off some prior work from @paoloricciuti in the pre-alpha version.

There are some optimizations I left for esbuild, which Vite (and thus SvelteKit) uses as the default minifier. These improvements were added 0.19.11 (evanw/esbuild#3568).

Copy link

changeset-bot bot commented Dec 27, 2023

🦋 Changeset detected

Latest commit: e7e1454

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

Copy link

vercel bot commented Dec 27, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
svelte-5-preview ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 8, 2024 6:12pm

@benmccann benmccann marked this pull request as draft December 27, 2023 22:34
@benmccann benmccann changed the title perf: hoist variables which are not mutated or reassigned perf: inline primitives which are not mutated or reassigned into template attributes Dec 28, 2023
@benmccann benmccann changed the title perf: inline primitives which are not mutated or reassigned into template attributes perf: use unchanged primitives directly in template attributes Dec 28, 2023
@Rich-Harris
Copy link
Member

I think we have to close this, sorry — the goal is to have a more holistic approach to hoisting that would result in fewer closures, and template optimisations would fall out of that. Better to wait until the codebase is stable enough to start that work in earnest rather than do it piecemeal and/or invest time in keeping this synced with main

@benmccann benmccann mentioned this pull request May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler Changes relating to the compiler perf
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants