From 144aec74d354385f35f486d996f14d8e9cdfdde3 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Mon, 6 May 2024 10:31:41 +0200 Subject: [PATCH 1/2] fix: allow to access private fields after `this` reassignment --- .changeset/gentle-toys-chew.md | 5 +++++ .../src/compiler/phases/2-analyze/index.js | 12 ++++++++++ .../3-transform/client/visitors/global.js | 12 ++++++++-- .../client/visitors/javascript-runes.js | 22 +++++++++++++++++-- .../svelte/src/compiler/phases/types.d.ts | 6 +++++ .../_config.js | 10 +++++++++ .../main.svelte | 17 ++++++++++++++ 7 files changed, 80 insertions(+), 4 deletions(-) create mode 100644 .changeset/gentle-toys-chew.md create mode 100644 packages/svelte/tests/runtime-runes/samples/class-private-fields-reassigned-this/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/class-private-fields-reassigned-this/main.svelte diff --git a/.changeset/gentle-toys-chew.md b/.changeset/gentle-toys-chew.md new file mode 100644 index 000000000000..dfc9f2013be5 --- /dev/null +++ b/.changeset/gentle-toys-chew.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: allow to access private fields after `this` reassignment diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index e4d6f6a4db9c..8e59bf632b02 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -1503,6 +1503,18 @@ const common_visitors = { return; } } + }, + ThisExpression(node, { next, path }) { + const parent = path.at(-1); + if (parent?.type === 'MemberExpression' && parent.object === node) { + return; + } + const class_declaration = path.find((path_element) => path_element.type === 'ClassDeclaration'); + if (class_declaration && class_declaration.type === 'ClassDeclaration') { + class_declaration.metadata = { + needs_private_getters: true + }; + } } }; diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js index 811407b35b84..4c8c61f62f3b 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js @@ -12,12 +12,20 @@ export const global_visitors = { return serialize_get_binding(node, state); } }, - MemberExpression(node, { state, next }) { + MemberExpression(node, { state, next, path }) { + const class_declaration = path.find((path_element) => path_element.type === 'ClassDeclaration'); + const needs_private_getters = + class_declaration?.type === 'ClassDeclaration' && + !!class_declaration.metadata?.needs_private_getters; + if (node.object.type === 'ThisExpression') { // rewrite `this.#foo` as `this.#foo.v` inside a constructor if (node.property.type === 'PrivateIdentifier') { const field = state.private_state.get(node.property.name); - if (field) { + if (field && needs_private_getters && state.in_constructor) { + return b.member(b.member(b.this, field.id), b.id('v')); + } + if (field && !needs_private_getters) { return state.in_constructor ? b.member(node, b.id('v')) : b.call('$.get', node); } } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js index 9f3192c638c7..56f4765b2907 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js @@ -8,7 +8,12 @@ import { regex_invalid_identifier_chars } from '../../../patterns.js'; /** @type {import('../types.js').ComponentVisitors} */ export const javascript_visitors_runes = { - ClassBody(node, { state, visit }) { + ClassBody(node, { state, visit, path }) { + const parent = path.at(-1); + + const needs_private_getters = + parent?.type === 'ClassDeclaration' && !!parent.metadata?.needs_private_getters; + /** @type {Map} */ const public_state = new Map(); @@ -64,6 +69,19 @@ export const javascript_visitors_runes = { } } + if (needs_private_getters) { + // each `#foo = $state()` needs a backing `#_foo` field is the class needs private getters + for (const [name, field] of private_state) { + let deconflicted = name; + while (private_ids.includes(deconflicted)) { + deconflicted = '_' + deconflicted; + } + + private_ids.push(deconflicted); + field.id = b.private_id(deconflicted); + } + } + // each `foo = $state()` needs a backing `#foo` field for (const [name, field] of public_state) { let deconflicted = name; @@ -121,7 +139,7 @@ export const javascript_visitors_runes = { value = b.call('$.source'); } - if (is_private) { + if (is_private && !needs_private_getters) { body.push(b.prop_def(field.id, value)); } else { // #foo; diff --git a/packages/svelte/src/compiler/phases/types.d.ts b/packages/svelte/src/compiler/phases/types.d.ts index bd6cad135a6d..7266ba080d4c 100644 --- a/packages/svelte/src/compiler/phases/types.d.ts +++ b/packages/svelte/src/compiler/phases/types.d.ts @@ -103,4 +103,10 @@ declare module 'estree' { scope: Scope; }; } + + interface ClassDeclaration { + metadata?: { + needs_private_getters: boolean; + }; + } } diff --git a/packages/svelte/tests/runtime-runes/samples/class-private-fields-reassigned-this/_config.js b/packages/svelte/tests/runtime-runes/samples/class-private-fields-reassigned-this/_config.js new file mode 100644 index 000000000000..440659df8896 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/class-private-fields-reassigned-this/_config.js @@ -0,0 +1,10 @@ +import { test } from '../../test'; + +export default test({ + compileOptions: { + dev: true + }, + async test({ assert, logs }) { + assert.deepEqual(logs, ['init', 1]); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/class-private-fields-reassigned-this/main.svelte b/packages/svelte/tests/runtime-runes/samples/class-private-fields-reassigned-this/main.svelte new file mode 100644 index 000000000000..1548672c0fdf --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/class-private-fields-reassigned-this/main.svelte @@ -0,0 +1,17 @@ + From 24ab91109ff5e01c966f0916c511fbfbcf5cd6b1 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 6 May 2024 15:55:09 +0200 Subject: [PATCH 2/2] simpler approach --- .../src/compiler/phases/2-analyze/index.js | 12 ---------- .../3-transform/client/visitors/global.js | 24 ++++++------------- .../client/visitors/javascript-runes.js | 22 ++--------------- .../svelte/src/compiler/phases/types.d.ts | 6 ----- .../_config.js | 2 +- .../main.svelte | 6 +++++ 6 files changed, 16 insertions(+), 56 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 8e59bf632b02..e4d6f6a4db9c 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -1503,18 +1503,6 @@ const common_visitors = { return; } } - }, - ThisExpression(node, { next, path }) { - const parent = path.at(-1); - if (parent?.type === 'MemberExpression' && parent.object === node) { - return; - } - const class_declaration = path.find((path_element) => path_element.type === 'ClassDeclaration'); - if (class_declaration && class_declaration.type === 'ClassDeclaration') { - class_declaration.metadata = { - needs_private_getters: true - }; - } } }; diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js index 4c8c61f62f3b..e229b5edf772 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js @@ -12,24 +12,14 @@ export const global_visitors = { return serialize_get_binding(node, state); } }, - MemberExpression(node, { state, next, path }) { - const class_declaration = path.find((path_element) => path_element.type === 'ClassDeclaration'); - const needs_private_getters = - class_declaration?.type === 'ClassDeclaration' && - !!class_declaration.metadata?.needs_private_getters; - - if (node.object.type === 'ThisExpression') { - // rewrite `this.#foo` as `this.#foo.v` inside a constructor - if (node.property.type === 'PrivateIdentifier') { - const field = state.private_state.get(node.property.name); - if (field && needs_private_getters && state.in_constructor) { - return b.member(b.member(b.this, field.id), b.id('v')); - } - if (field && !needs_private_getters) { - return state.in_constructor ? b.member(node, b.id('v')) : b.call('$.get', node); - } + MemberExpression(node, { state, next }) { + // rewrite `this.#foo` as `this.#foo.v` inside a constructor + if (node.property.type === 'PrivateIdentifier') { + const field = state.private_state.get(node.property.name); + if (field) { + return state.in_constructor ? b.member(node, b.id('v')) : b.call('$.get', node); } - + } else if (node.object.type === 'ThisExpression') { // rewrite `this.foo` as `this.#foo.v` inside a constructor if (node.property.type === 'Identifier' && !node.computed) { const field = state.public_state.get(node.property.name); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js index 56f4765b2907..9f3192c638c7 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js @@ -8,12 +8,7 @@ import { regex_invalid_identifier_chars } from '../../../patterns.js'; /** @type {import('../types.js').ComponentVisitors} */ export const javascript_visitors_runes = { - ClassBody(node, { state, visit, path }) { - const parent = path.at(-1); - - const needs_private_getters = - parent?.type === 'ClassDeclaration' && !!parent.metadata?.needs_private_getters; - + ClassBody(node, { state, visit }) { /** @type {Map} */ const public_state = new Map(); @@ -69,19 +64,6 @@ export const javascript_visitors_runes = { } } - if (needs_private_getters) { - // each `#foo = $state()` needs a backing `#_foo` field is the class needs private getters - for (const [name, field] of private_state) { - let deconflicted = name; - while (private_ids.includes(deconflicted)) { - deconflicted = '_' + deconflicted; - } - - private_ids.push(deconflicted); - field.id = b.private_id(deconflicted); - } - } - // each `foo = $state()` needs a backing `#foo` field for (const [name, field] of public_state) { let deconflicted = name; @@ -139,7 +121,7 @@ export const javascript_visitors_runes = { value = b.call('$.source'); } - if (is_private && !needs_private_getters) { + if (is_private) { body.push(b.prop_def(field.id, value)); } else { // #foo; diff --git a/packages/svelte/src/compiler/phases/types.d.ts b/packages/svelte/src/compiler/phases/types.d.ts index 7266ba080d4c..bd6cad135a6d 100644 --- a/packages/svelte/src/compiler/phases/types.d.ts +++ b/packages/svelte/src/compiler/phases/types.d.ts @@ -103,10 +103,4 @@ declare module 'estree' { scope: Scope; }; } - - interface ClassDeclaration { - metadata?: { - needs_private_getters: boolean; - }; - } } diff --git a/packages/svelte/tests/runtime-runes/samples/class-private-fields-reassigned-this/_config.js b/packages/svelte/tests/runtime-runes/samples/class-private-fields-reassigned-this/_config.js index 440659df8896..88b806c0f085 100644 --- a/packages/svelte/tests/runtime-runes/samples/class-private-fields-reassigned-this/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/class-private-fields-reassigned-this/_config.js @@ -5,6 +5,6 @@ export default test({ dev: true }, async test({ assert, logs }) { - assert.deepEqual(logs, ['init', 1]); + assert.deepEqual(logs, ['init', 1, 'init', 1]); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/class-private-fields-reassigned-this/main.svelte b/packages/svelte/tests/runtime-runes/samples/class-private-fields-reassigned-this/main.svelte index 1548672c0fdf..fad50c5fe19c 100644 --- a/packages/svelte/tests/runtime-runes/samples/class-private-fields-reassigned-this/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/class-private-fields-reassigned-this/main.svelte @@ -10,8 +10,14 @@ get count(){ return this.#count; } + + get count2() { + const instance = this; + return instance.#count; + } } const counter = new Counter(); $inspect(counter.count) + $inspect(counter.count2)