8000 feat: State declarations in class constructors (#15820) · sveltejs/svelte@d103adf · GitHub
[go: up one dir, main page]
Skip to content

Commit d103adf

Browse files
elliott-with-the-longest-name-on-githubRich-Harris
elliott-with-the-longest-name-on-github
and
Rich-Harris
authored
feat: State declarations in class constructors (#15820)
* feat: State declarations in class constructors * feat: Analysis phase * misc * feat: client * improvements * feat: It is now at least backwards compatible. though the new stuff may still be wrong * feat: It works I think? * final cleanup?? * tests * test for better types * changeset * rename functions (the function doesn't test call-expression-ness) * small readability tweak * failing test * fix * disallow computed state fields * tweak message to better accommodate the case in which state is declared after a regular property * failing test * wildly confusing to have so many things called 'class analysis' - rename the transform-phrase ones to transformers * missed a spot * and another * store analysis for use during transformation * move code to where it is used * do the analysis upfront, it's way simpler * skip failing test for now * simplify * get rid of the class * on second thoughts * reduce indirection * make analysis available at transform time * WIP * WIP * WIP * fix * remove unused stuff * revert snapshot tests * unused * note to self * fix * unused * unused * remove some unused stuff * unused * lint, tidy up * reuse helper * tweak * simplify/DRY * unused * tweak * unused * more * tweak * tweak * fix proxying logic * tweak * tweak * adjust message to accommodate more cases * unskip and fix test * fix * move * revert unneeded drive-by change * fix * fix * update docs --------- Co-authored-by: Rich Harris <rich.harris@vercel.com>
1 parent 42e7e81 commit d103adf

File tree

73 files changed

+1070
-363
lines changed
  • class-state-constructor-9
  • const-tag-invalid-rune-usage

    • Some content is hidden

    Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

    73 files changed

    +1070
    -363
    lines changed

    ‎.changeset/mean-squids-scream.md

    Lines changed: 5 additions & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -0,0 +1,5 @@
    1+
    ---
    2+
    'svelte': minor
    3+
    ---
    4+
    5+
    feat: allow state fields to be declared inside class constructors

    ‎documentation/docs/02-runes/02-$state.md

    Lines changed: 3 additions & 5 deletions
    Original file line numberDiff line numberDiff line change
    @@ -67,16 +67,15 @@ todos[0].done = !todos[0].done;
    6767

    6868
    ### Classes
    6969

    70-
    You can also use `$state` in class fields (whether public or private):
    70+
    You can also use `$state` in class fields (whether public or private), or as the first assignment to a property immediately inside the `constructor`:
    7171

    7272
    ```js
    7373
    // @errors: 7006 2554
    7474
    class Todo {
    7575
    done = $state(false);
    76-
    text = $state();
    7776

    7877
    constructor(text) {
    79-
    this.text = text;
    78+
    this.text = $state(text);
    8079
    }
    8180

    8281
    reset() {
    @@ -110,10 +109,9 @@ You can either use an inline function...
    110109
    // @errors: 7006 2554
    111110
    class Todo {
    112111
    done = $state(false);
    113-
    text = $state();
    114112

    115113
    constructor(text) {
    116-
    this.text = text;
    114+
    this.text = $state(text);
    117115
    }
    118116

    119117
    +++reset = () => {+++

    ‎documentation/docs/98-reference/.generated/compile-errors.md

    Lines changed: 33 additions & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -846,6 +846,38 @@ Cannot reassign or bind to snippet parameter
    846846
    This snippet is shadowing the prop `%prop%` with the same name
    847847
    ```
    848848

    849+
    ### state_field_duplicate
    850+
    851+
    ```
    852+
    `%name%` has already been declared on this class
    853+
    ```
    854+
    855+
    An assignment to a class field that uses a `$state` or `$derived` rune is considered a _state field declaration_. The declaration can happen in the class body...
    856+
    857+
    ```js
    858+
    class Counter {
    859+
    count = $state(0);
    860+
    }
    861+
    ```
    862+
    863+
    ...or inside the constructor...
    864+
    865+
    ```js
    866+
    class Counter {
    867+
    constructor() {
    868+
    this.count = $state(0);
    869+
    }
    870+
    }
    871+
    ```
    872+
    873+
    ...but it can only happen once.
    874+
    875+
    ### state_field_invalid_assignment
    876+
    877+
    ```
    878+
    Cannot assign to a state field before its declaration
    879+
    ```
    880+
    849881
    ### state_invalid_export
    850882

    851883
    ```
    @@ -855,7 +887,7 @@ Cannot export state from a module if it is reassigned. Either export a function
    855887
    ### state_invalid_placement
    856888

    857889
    ```
    858-
    `%rune%(...)` can only be used as a variable declaration initializer or a class field
    890+
    `%rune%(...)` can only be used as a variable declaration initializer, a class field declaration, or the first assignment to a class field at the top level of the constructor.
    859891
    ```
    860892

    861893
    ### store_invalid_scoped_subscription

    ‎packages/svelte/messages/compile-errors/script.md

    Lines changed: 29 additions & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -212,13 +212,41 @@ It's possible to export a snippet from a `<script module>` block, but only if it
    212212

    213213
    > Cannot reassign or bind to snippet parameter
    214214
    215+
    ## state_field_duplicate
    216+
    217+
    > `%name%` has already been declared on this class
    218+
    219+
    An assignment to a class field that uses a `$state` or `$derived` rune is considered a _state field declaration_. The declaration can happen in the class body...
    220+
    221+
    ```js
    222+
    class Counter {
    223+
    count = $state(0);
    224+
    }
    225+
    ```
    226+
    227+
    ...or inside the constructor...
    228+
    229+
    ```js
    230+
    class Counter {
    231+
    constructor() {
    232+
    this.count = $state(0);
    233+
    }
    234+
    }
    235+
    ```
    236+
    237+
    ...but it can only happen once.
    238+
    239+
    ## state_field_invalid_assignment
    240+
    241+
    > Cannot assign to a state field before its declaration
    242+
    215243
    ## state_invalid_export
    216244

    217245
    > Cannot export state from a module if it is reassigned. Either export a function returning the state value or only mutate the state value's properties
    218246
    219247
    ## state_invalid_placement
    220248

    221-
    > `%rune%(...)` can only be used as a variable declaration initializer or a class field
    249+
    > `%rune%(...)` can only be used as a variable declaration initializer, a class field declaration, or the first assignment to a class field at the top level of the constructor.
    222250
    223251
    ## store_invalid_scoped_subscription
    224252

    ‎packages/svelte/src/compiler/errors.js

    Lines changed: 21 additions & 2 deletions
    Original file line numberDiff line numberDiff line change
    @@ -461,6 +461,25 @@ export function snippet_parameter_assignment(node) {
    461461
    e(node, 'snippet_parameter_assignment', `Cannot reassign or bind to snippet parameter\nhttps://svelte.dev/e/snippet_parameter_assignment`);
    462462
    }
    463463

    464+
    /**
    465+
    * `%name%` has already been declared on this class
    466+
    * @param {null | number | NodeLike} node
    467+
    * @param {string} name
    468+
    * @returns {never}
    469+
    */
    470+
    export function state_field_duplicate(node, name) {
    471+
    e(node, 'state_field_duplicate', `\`${name}\` has already been declared on this class\nhttps://svelte.dev/e/state_field_duplicate`);
    472+
    }
    473+
    474+
    /**
    475+
    * Cannot assign to a state field before its declaration
    476+
    * @param {null | number | NodeLike} node
    477+
    * @returns {never}
    478+
    */
    479+
    export function state_field_invalid_assignment(node) {
    480+
    e(node, 'state_field_invalid_assignment', `Cannot assign to a state field before its declaration\nhttps://svelte.dev/e/state_field_invalid_assignment`);
    481+
    }
    482+
    464483
    /**
    465484
    * Cannot export state from a module if it is reassigned. Either export a function returning the state value or only mutate the state value's properties
    466485
    * @param {null | number | NodeLike} node
    @@ -471,13 +490,13 @@ export function state_invalid_export(node) {
    471490
    }
    472491

    473492
    /**
    474-
    * `%rune%(...)` can only be used as a variable declaration initializer or a class field
    493+
    * `%rune%(...)` can only be used as a variable declaration initializer, a class field declaration, or the first assignment to a class field at the top level of the constructor.
    475494
    * @param {null | number | NodeLike} node
    476495
    * @param {string} rune
    477496
    * @returns {never}
    478497
    */
    479498
    export function state_invalid_placement(node, rune) {
    480-
    e(node, 'state_invalid_placement', `\`${rune}(...)\` can only be used as a variable declaration initializer or a class field\nhttps://svelte.dev/e/state_invalid_placement`);
    499+
    e(node, 'state_invalid_placement', `\`${rune}(...)\` can only be used as a variable declaration initializer, a class field declaration, or the first assignment to a class field at the top level of the constructor.\nhttps://svelte.dev/e/state_invalid_placement`);
    481500
    }
    482501

    483502
    /**

    ‎packages/svelte/src/compiler/phases/2-analyze/index.js

    Lines changed: 8 additions & 4 deletions
    Original file line numberDiff line numberDiff line change
    @@ -48,6 +48,7 @@ import { Literal } from './visitors/Literal.js';
    4848
    import { MemberExpression } from './visitors/MemberExpression.js';
    4949
    import { NewExpression } from './visitors/NewExpression.js';
    5050
    import { OnDirective } from './visitors/OnDirective.js';
    51+
    import { PropertyDefinition } from './visitors/PropertyDefinition.js';
    5152
    import { RegularElement } from './visitors/RegularElement.js';
    5253
    import { RenderTag } from './visitors/RenderTag.js';
    5354
    import { SlotElement } from './visitors/SlotElement.js';
    @@ -164,6 10000 +165,7 @@ const visitors = {
    164165
    MemberExpression,
    165166
    NewExpression,
    166167
    OnDirective,
    168+
    PropertyDefinition,
    167169
    RegularElement,
    168170
    RenderTag,
    169171
    SlotElement,
    @@ -256,7 +258,8 @@ export function analyze_module(ast, options) {
    256258
    accessors: false,
    257259
    runes: true,
    258260
    immutable: true,
    259-
    tracing: false
    261+
    tracing: false,
    262+
    classes: new Map()
    260263
    };
    261264

    262265
    walk(
    @@ -265,7 +268,7 @@ export function analyze_module(ast, options) {
    265268
    scope,
    266269
    scopes,
    267270
    analysis: /** @type {ComponentAnalysis} */ (analysis),
    268-
    derived_state: [],
    271+
    state_fields: new Map(),
    269272
    // TODO the following are not needed for modules, but we have to pass them in order to avoid type error,
    270273
    // and reducing the type would result in a lot of tedious type casts elsewhere - find a good solution one day
    271274
    ast_type: /** @type {any} */ (null),
    @@ -429,6 +432,7 @@ export function analyze_component(root, source, options) {
    429432
    elements: [],
    430433
    runes,
    431434
    tracing: false,
    435+
    classes: new Map(),
    432436
    immutable: runes || options.immutable,
    433437
    exports: [],
    434438
    uses_props: false,
    @@ -624,7 +628,7 @@ export function analyze_component(root, source, options) {
    624628
    has_props_rune: false,
    625629
    component_slots: new Set(),
    626630
    expression: null,
    627-
    derived_state: [],
    631+
    state_fields: new Map(),
    628632
    function_depth: scope.function_depth,
    629633
    reactive_statement: null
    630634
    };
    @@ -691,7 +695,7 @@ export function analyze_component(root, source, options) {
    691695
    reactive_statement: null,
    692696
    component_slots: new Set(),
    693697
    expression: null,
    694-
    derived_state: [],
    698+
    state_fields: new Map(),
    695699
    function_depth: scope.function_depth
    696700
    };
    697701

    ‎packages/svelte/src/compiler/phases/2-analyze/types.d.ts

    Lines changed: 5 additions & 2 deletions
    Original file line numberDiff line numberDiff line change
    @@ -1,6 +1,6 @@
    11
    import type { Scope } from '../scope.js';
    22
    import type { ComponentAnalysis, ReactiveStatement } from '../types.js';
    3-
    import type { AST, ExpressionMetadata, ValidatedCompileOptions } from '#compiler';
    3+
    import type { AST, ExpressionMetadata, StateField, ValidatedCompileOptions } from '#compiler';
    44

    55
    export interface AnalysisState {
    66
    scope: Scope;
    @@ -18,7 +18,10 @@ export interface AnalysisState {
    1818
    component_slots: Set<string>;
    1919
    /** Information about the current expression/directive/block value */
    2020
    expression: ExpressionMetadata | null;
    21-
    derived_state: { name: string; private: boolean }[];
    21+
    22+
    /** Used to analyze class state */
    23+
    state_fields: Map<string, StateField>;
    24+
    2225
    function_depth: number;
    2326

    2427
    // legacy stuff

    ‎packages/svelte/src/compiler/phases/2-analyze/visitors/AssignmentExpression.js

    Lines changed: 1 addition & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -8,7 +8,7 @@ import { validate_assignment } from './shared/utils.js';
    88
    * @param {Context} context
    99
    */
    1010
    export function AssignmentExpression(node, context) {
    11-
    validate_assignment(node, node.left, context.state);
    11+
    validate_assignment(node, node.left, context);
    1212

    1313
    if (context.state.reactive_statement) {
    1414
    const id = node.left.type === 'MemberExpression' ? object(node.left) : node.left;

    ‎packages/svelte/src/compiler/phases/2-analyze/visitors/BindDirective.js

    Lines changed: 1 addition & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -158,7 +158,7 @@ export function BindDirective(node, context) {
    158158
    return;
    159159
    }
    160160

    161-
    validate_assignment(node, node.expression, context.state);
    161+
    validate_assignment(node, node.expression, context);
    162162

    163163
    const assignee = node.expression;
    164164
    const left = object(assignee);

    ‎packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js

    Lines changed: 45 additions & 6 deletions
    Original file line numberDiff line numberDiff line change
    @@ -114,12 +114,13 @@ export function CallExpression(node, context) {
    114114
    case '$state':
    115115
    case '$state.raw':
    116116
    case '$derived':
    117-
    case '$derived.by':
    118-
    if (
    119-
    (parent.type !== 'VariableDeclarator' ||
    120-
    get_parent(context.path, -3).type === 'ConstTag') &&
    121-
    !(parent.type === 'PropertyDefinition' && !parent.static && !parent.computed)
    122-
    ) {
    117+
    case '$derived.by': {
    118+
    const valid =
    119+
    is_variable_declaration(parent, context) ||
    120+
    is_class_property_definition(parent) ||
    121+
    is_class_property_assignment_at_constructor_root(parent, context);
    122+
    123+
    if (!valid) {
    123124
    e.state_invalid_placement(node, rune);
    124125
    }
    125126

    @@ -130,6 +131,7 @@ export function CallExpression(node, context) {
    130131
    }
    131132

    132133
    break;
    134+
    }
    133135

    134136
    case '$effect':
    135137
    case '$effect.pre':
    @@ -270,3 +272,40 @@ function get_function_label(nodes) {
    270272
    return parent.id.name;
    271273
    }
    272274
    }
    275+
    276+
    /**
    277+
    * @param {AST.SvelteNode} parent
    278+
    * @param {Context} context
    279+
    */
    280+
    function is_variable_declaration(parent, context) {
    281+
    return parent.type === 'VariableDeclarator' && get_parent(context.path, -3).type !== 'ConstTag';
    282+
    }
    283+
    284+
    /**
    285+
    * @param {AST.SvelteNode} parent
    286+
    */
    287+
    function is_class_property_definition(parent) {
    288+
    return parent.type === 'PropertyDefinition' && !parent.static && !parent.computed;
    289+
    }
    290+
    291+
    /**
    292+
    * @param {AST.SvelteNode} node
    293+
    * @param {Context} context
    294+
    */
    295+
    function is_class_property_assignment_at_constructor_root(node, context) {
    296+
    if (
    297+
    node.type === 'AssignmentExpression' &&
    298+
    node.operator === '=' &&
    299+
    node.left.type === 'MemberExpression' &&
    300+
    node.left.object.type === 'ThisExpression' &&
    301+
    ((node.left.property.type === 'Identifier' && !node.left.computed) ||
    302+
    node.left.property.type === 'PrivateIdentifier' ||
    303+
    node.left.property.type === 'Literal')
    304+
    ) {
    305+
    // MethodDefinition (-5) -> FunctionExpression (-4) -> BlockStatement (-3) -> ExpressionStatement (-2) -> AssignmentExpression (-1)
    306+
    const parent = get_parent(context.path, -5);
    307+
    return parent?.type === 'MethodDefinition' && parent.kind === 'constructor';
    308+
    }
    309+
    310+
    return false;
    311+
    }

    0 commit comments

    Comments
     (0)
    0