Skip to content

fix: improve partial evaluation #15781

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

Merged
merged 26 commits into from
Apr 18, 2025

Conversation

Ocean-OS
Copy link
Contributor

@Ocean-OS Ocean-OS commented Apr 17, 2025

This further improves the partial evaluation implemented in #15494. It accounts for sources that are declared as $state, but never reassigned to (and can be treated as constants), and other (somewhat more complex) cases.

A few possible end goals for this:

  • Respect TypeScript type annotations/assertions
  • Primitive function type inference/evaluation?:
function get_pi() {
    return 3.14159265358979; //constant value
}
  • In call expressions, infer types if the callee is a known global:
String(stuff); //string type
Math.random(); //number

closes #14280

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
  • Prefix your PR title with feat:, fix:, chore:, or docs:.
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.
  • If this PR changes code within packages/svelte/src, add a changeset (npx changeset).

Tests and linting

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

Copy link

changeset-bot bot commented Apr 17, 2025

🦋 Changeset detected

Latest commit: 0ef68b9

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

This PR includes changesets to release 1 package
Name Type
svelte Minor

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

@svelte-docs-bot
Copy link

Copy link
Contributor

Playground

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

@Rich-Harris
Copy link
Member

This is great, thank you! Hope you don't mind me jumping in, I started out with some minor comments but got carried away. Am keen to get this in to bring us slightly closer to #15292 (comment) — was there anything else you had planned for this PR or is it ready to go in its current form?

@Ocean-OS
Copy link
Contributor Author

I was thinking of maybe looking into some of the goals I outlined, such as adding more globals and maybe some function analysis, how does that sound?

@Rich-Harris
Copy link
Member

Adding more globals sounds good. I feel like function analysis could potentially become a bit of a rabbit hole with diminishing returns; maybe if you have something in mind it could be a good follow-up PR though?

@Ocean-OS
Copy link
Contributor Author

Ocean-OS commented Apr 18, 2025

I started working on function analysis last night, I'll try pushing it (along with more globals and a couple of other smallish things) now and if there are too many possible issues, I'll remove it (and maybe work on it in a follow-up PR)

@Ocean-OS
Copy link
Contributor Author

That's not too bad, I honestly expected more failing tests.
It appears that some of the tests failed just because the evaluation worked (and they were snapshots that didn't expect the change), but there are definitely some actually failing tests.

@Ocean-OS
Copy link
Contributor Author

After looking more into the failing tests, I've decided against trying to work on this in this PR, but since it does seem rather promising, I'll work on it in another PR, probably after #15792 gets merged.

@Ocean-OS
Copy link
Contributor Author

Currently the only failing test has to do with sourcemaps, and I don't quite understand it, could you please fix it?

@Rich-Harris
Copy link
Member

looking into it

@Rich-Harris Rich-Harris marked this pull request as ready for review April 18, 2025 15:17
@Rich-Harris Rich-Harris merged commit d0dcc0b into sveltejs:main Apr 18, 2025
9 checks passed
@github-actions github-actions bot mentioned this pull request Apr 18, 2025
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.

Non-updated state treated differently in different places
2 participants