-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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: add partial evaluation #15494
base: main
Are you sure you want to change the base?
feat: add partial evaluation #15494
Conversation
🦋 Changeset detectedLatest commit: 696bb2e 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 |
|
Marking as ready for review, even though there's room for future improvement:
|
Is this PR meant to be combined with #15374, or just replace it? |
I think it's probably a replacement given their overlap. Which isn't a commentary on your PR! Convergent evolution; I started working on this before I understood the degree to which we were addressing a similar problem. The reason I favour this PR over #15374 is that it feels more future-proof to have an Which isn't to say this analysis is particularly sophisticated — it definitely isn't, compared to what a good general purpose optimiser will give you. But it gives us something to build from. (Maybe one day we'll be able to incorporate type information and so on.) |
Noticed while looking into something unrelated that this input...
...yields this output:
At minimum, it should be this instead:
But we can go much further. Something I've been meaning to get round to for a while is to add some simple partial evaluation, which would make it possible to avoid the
?? ''
stuff in a lot more cases. @Ocean-OS made some progress on this front in #15374 but I'd like it to be able to accommodate bindings as well (in other words, knowing thatmessage
is always a string andcount
is always a number). I think it makes sense to implement this as anevaluate
method on theScope
class.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