-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat: State declarations in class constructors #15820
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
Open
elliott-with-the-longest-name-on-github
wants to merge
57
commits into
main
Choose a base branch
from
elliott/class-constructor-state
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+991
−348
Open
Changes from all commits
Commits
Show all changes
57 commits
Select commit
Hold shift + click to select a range
134d435
feat: State declarations in class constructors
elliott-with-the-longest-name-on-github fb8d6d7
feat: Analysis phase
elliott-with-the-longest-name-on-github 033a466
misc
elliott-with-the-longest-name-on-github 005ba29
feat: client
elliott-with-the-longest-name-on-github 4d6422c
improvements
elliott-with-the-longest-name-on-github adb6e71
feat: It is now at least backwards compatible. though the new stuff m…
elliott-with-the-longest-name-on-github 92940ff
feat: It works I think?
elliott-with-the-longest-name-on-github ac42ad5
final cleanup??
elliott-with-the-longest-name-on-github b44eed9
tests
elliott-with-the-longest-name-on-github 12a02b7
test for better types
elliott-with-the-longest-name-on-github 4a19fd1
Merge branch 'main' into elliott/class-constructor-state
Rich-Harris 50adbfb
changeset
Rich-Harris 682b0e6
rename functions (the function doesn't test call-expression-ness)
Rich-Harris 76b07e5
small readability tweak
Rich-Harris 6395085
failing test
Rich-Harris 0024e1e
fix
Rich-Harris 8c7ad3c
disallow computed state fields
Rich-Harris 15c7b14
tweak message to better accommodate the case in which state is declar…
Rich-Harris 0ac22c1
failing test
Rich-Harris 4edebbc
wildly confusing to have so many things called 'class analysis' - ren…
Rich-Harris 1e0f423
missed a spot
Rich-Harris f81405c
and another
Rich-Harris 41e8ade
store analysis for use during transformation
Rich-Harris 1d1f0eb
move code to where it is used
Rich-Harris 823e66f
do the analysis upfront, it's way simpler
Rich-Harris 0dcd3cd
skip failing test for now
Rich-Harris 7341f40
simplify
Rich-Harris 75680a9
get rid of the class
Rich-Harris 2ffb863
on second thoughts
Rich-Harris b1e095a
reduce indirection
Rich-Harris c407dc0
make analysis available at transform time
Rich-Harris 2efb766
WIP
Rich-Harris 0e9ca23
WIP
Rich-Harris b7bbae8
WIP
Rich-Harris 737bfb5
fix
Rich-Harris c536ec6
remove unused stuff
Rich-Harris fbf1b4e
revert snapshot tests
Rich-Harris de015b0
unused
Rich-Harris 4653f54
note to self
Rich-Harris 23f269b
fix
Rich-Harris d1bb2c9
unused
Rich-Harris de77e69
unused
Rich-Harris dfb4e13
remove some unused stuff
Rich-Harris 2556031
unused
Rich-Harris c7e8422
lint, tidy up
Rich-Harris 1a78e27
reuse helper
Rich-Harris 90e7e0d
tweak
Rich-Harris a6cc07c
simplify/DRY
Rich-Harris 45115de
unused
Rich-Harris 56bd834
tweak
Rich-Harris 855f209
unused
Rich-Harris d31831b
more
Rich-Harris 0cac3d2
tweak
Rich-Harris b1cc424
tweak
Rich-Harris bbf0191
fix proxying logic
Rich-Harris 2b42182
tweak
Rich-Harris 63e60c7
tweak
Rich-Harris File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'svelte': minor | ||
--- | ||
|
||
feat: allow state fields to be declared inside class constructors |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
114 changes: 100 additions & 14 deletions
114
packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,30 +1,116 @@ | ||
/** @import { ClassBody } from 'estree' */ | ||
/** @import { AssignmentExpression, CallExpression, ClassBody, PropertyDefinition, Expression, PrivateIdentifier, MethodDefinition } from 'estree' */ | ||
/** @import { StateField } from '#compiler' */ | ||
/** @import { Context } from '../types' */ | ||
import * as b from '#compiler/builders'; | ||
import { get_rune } from '../../scope.js'; | ||
import * as e from '../../../errors.js'; | ||
import { is_state_creation_rune } from '../../../../utils.js'; | ||
import { get_name } from '../../nodes.js'; | ||
import { regex_invalid_identifier_chars } from '../../patterns.js'; | ||
|
||
/** | ||
* @param {ClassBody} node | ||
* @param {Context} context | ||
*/ | ||
export function ClassBody(node, context) { | ||
/** @type {{name: string, private: boolean}[]} */ | ||
const derived_state = []; | ||
if (!context.state.analysis.runes) { | ||
context.next(); | ||
return; | ||
} | ||
|
||
/** @type {string[]} */ | ||
const private_ids = []; | ||
|
||
for (const definition of node.body) { | ||
for (const prop of node.body) { | ||
if ( | ||
definition.type === 'PropertyDefinition' && | ||
(definition.key.type === 'PrivateIdentifier' || definition.key.type === 'Identifier') && | ||
definition.value?.type === 'CallExpression' | ||
(prop.type === 'MethodDefinition' || prop.type === 'PropertyDefinition') && | ||
prop.key.type === 'PrivateIdentifier' | ||
) { | ||
const rune = get_rune(definition.value, context.state.scope); | ||
if (rune === '$derived' || rune === '$derived.by') { | ||
derived_state.push({ | ||
name: definition.key.name, | ||
private: definition.key.type === 'PrivateIdentifier' | ||
}); | ||
private_ids.push(prop.key.name); | ||
} | ||
} | ||
|
||
/** @type {Record<string, StateField>} */ | ||
const state_fields = {}; | ||
|
||
context.state.analysis.classes.set(node, state_fields); | ||
|
||
/** @type {string[]} */ | ||
const seen = []; | ||
|
||
/** @type {MethodDefinition | null} */ | ||
let constructor = null; | ||
|
||
/** | ||
* @param {PropertyDefinition | AssignmentExpression} node | ||
* @param {Expression | PrivateIdentifier} key | ||
* @param {Expression | null | undefined} value | ||
*/ | ||
function handle(node, key, value) { | ||
const name = get_name(key); | ||
if (name === null) return; | ||
|
||
const rune = get_rune(value, context.state.scope); | ||
|
||
if (rune && is_state_creation_rune(rune)) { | ||
if (seen.includes(name)) { | ||
e.constructor_state_reassignment(node); // TODO the same thing applies to duplicate fields, so the code/message needs to change | ||
} | ||
|
||
state_fields[name] = { | ||
node, | ||
type: rune, | ||
// @ts-expect-error for public state this is filled out in a moment | ||
key: key.type === 'PrivateIdentifier' ? key : null, | ||
value: /** @type {CallExpression} */ (value) | ||
}; | ||
} | ||
|
||
if (value) { | ||
seen.push(name); | ||
} | ||
} | ||
|
||
for (const child of node.body) { | ||
if (child.type === 'PropertyDefinition' && !child.computed) { | ||
handle(child, child.key, child.value); | ||
} | ||
|
||
if (child.type === 'MethodDefinition' && child.kind === 'constructor') { | ||
constructor = child; | ||
} | ||
} | ||
|
||
if (constructor) { | ||
for (const statement of constructor.value.body.body) { | ||
if (statement.type !== 'ExpressionStatement') continue; | ||
if (statement.expression.type !== 'AssignmentExpression') continue; | ||
|
||
const { left, right } = statement.expression; | ||
|
||
if (left.type !== 'MemberExpression') continue; | ||
if (left.object.type !== 'ThisExpression') continue; | ||
if (left.computed && left.property.type !== 'Literal') continue; | ||
|
||
handle(statement.expression, left.property, right); | ||
} | ||
} | ||
|
||
for (const name in state_fields) { | ||
if (name[0] === '#') { | ||
continue; | ||
} | ||
|
||
const field = state_fields[name]; | ||
|
||
let deconflicted = name.replace(regex_invalid_identifier_chars, '_'); | ||
while (private_ids.includes(deconflicted)) { | ||
deconflicted = '_' + deconflicted; | ||
} | ||
|
||
private_ids.push(deconflicted); | ||
field.key = b.private_id(deconflicted); | ||
} | ||
|
||
context.next({ ...context.state, derived_state }); | ||
context.next({ ...context.state, state_fields }); | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
note to self