less invasive approach

pull/9669/head
Rich Harris 3 years ago
parent 45dc56b37d
commit 289d568357

@ -38,7 +38,7 @@ function js(script, root, allow_reactive_declarations, parent) {
body: [] body: []
}; };
const { scope, scopes } = create_scopes(ast, root, allow_reactive_declarations, false, parent); const { scope, scopes } = create_scopes(ast, root, allow_reactive_declarations, parent);
return { ast, scope, scopes }; return { ast, scope, scopes };
} }
@ -191,7 +191,7 @@ function get_delegated_event(node, context) {
* @returns {import('../types.js').Analysis} * @returns {import('../types.js').Analysis}
*/ */
export function analyze_module(ast, options) { export function analyze_module(ast, options) {
const { scope, scopes } = create_scopes(ast, new ScopeRoot(), false, false, null); const { scope, scopes } = create_scopes(ast, new ScopeRoot(), false, null);
for (const [name, references] of scope.references) { for (const [name, references] of scope.references) {
if (name[0] !== '$' || ReservedKeywords.includes(name)) continue; if (name[0] !== '$' || ReservedKeywords.includes(name)) continue;
@ -242,7 +242,7 @@ export function analyze_component(root, options) {
const module = js(root.module, scope_root, false, null); const module = js(root.module, scope_root, false, null);
const instance = js(root.instance, scope_root, true, module.scope); const instance = js(root.instance, scope_root, true, module.scope);
const { scope, scopes } = create_scopes(root.fragment, scope_root, false, true, instance.scope); const { scope, scopes } = create_scopes(root.fragment, scope_root, false, instance.scope);
/** @type {import('../types.js').Template} */ /** @type {import('../types.js').Template} */
const template = { ast: root.fragment, scope, scopes }; const template = { ast: root.fragment, scope, scopes };
@ -416,9 +416,24 @@ export function analyze_component(root, options) {
// warn on any nonstate declarations that are a) mutated and b) referenced in the template // warn on any nonstate declarations that are a) mutated and b) referenced in the template
for (const scope of [module.scope, instance.scope]) { for (const scope of [module.scope, instance.scope]) {
for (const [name, binding] of scope.declarations) { outer: for (const [name, binding] of scope.declarations) {
if (binding.kind === 'normal' && binding.mutated && binding.referenced_in_template) { if (binding.kind === 'normal' && binding.mutated) {
warn(warnings, binding.node, [], 'non-state-reference', name); for (const { path } of binding.references) {
if (path[0].type !== 'Fragment') continue;
for (let i = 1; i < path.length; i += 1) {
const type = path[i].type;
if (
type === 'FunctionDeclaration' ||
type === 'FunctionExpression' ||
type === 'ArrowFunctionExpression'
) {
continue;
}
}
warn(warnings, binding.node, [], 'non-state-reference', name);
break outer;
}
} }
} }
} }

@ -169,9 +169,8 @@ export class Scope {
/** /**
* @param {import('estree').Identifier} node * @param {import('estree').Identifier} node
* @param {import('#compiler').SvelteNode[]} path * @param {import('#compiler').SvelteNode[]} path
* @param {boolean} is_template
*/ */
reference(node, path, is_template) { reference(node, path) {
let references = this.references.get(node.name); let references = this.references.get(node.name);
if (!references) this.references.set(node.name, (references = [])); if (!references) this.references.set(node.name, (references = []));
@ -179,10 +178,9 @@ export class Scope {
const binding = this.declarations.get(node.name); const binding = this.declarations.get(node.name);
if (binding) { if (binding) {
if (is_template) binding.referenced_in_template = true;
binding.references.push({ node, path }); binding.references.push({ node, path });
} else if (this.#parent) { } else if (this.#parent) {
this.#parent.reference(node, path, is_template); this.#parent.reference(node, path);
} else { } else {
// no binding was found, and this is the top level scope, // no binding was found, and this is the top level scope,
// which means this is a global // which means this is a global
@ -217,11 +215,10 @@ export class ScopeRoot {
* @param {import('#compiler').SvelteNode} ast * @param {import('#compiler').SvelteNode} ast
* @param {ScopeRoot} root * @param {ScopeRoot} root
* @param {boolean} allow_reactive_declarations * @param {boolean} allow_reactive_declarations
* @param {boolean} is_template
* @param {Scope | null} parent * @param {Scope | null} parent
*/ */
export function create_scopes(ast, root, allow_reactive_declarations, is_template, parent) { export function create_scopes(ast, root, allow_reactive_declarations, parent) {
/** @typedef {{ scope: Scope, is_template: boolean }} State */ /** @typedef {{ scope: Scope }} State */
/** /**
* A map of node->associated scope. A node appearing in this map does not necessarily mean that it created a scope * A map of node->associated scope. A node appearing in this map does not necessarily mean that it created a scope
@ -232,9 +229,9 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat
scopes.set(ast, scope); scopes.set(ast, scope);
/** @type {State} */ /** @type {State} */
const state = { scope, is_template }; const state = { scope };
/** @type {[Scope, { node: import('estree').Identifier; path: import('#compiler').SvelteNode[]; is_template: boolean }][]} */ /** @type {[Scope, { node: import('estree').Identifier; path: import('#compiler').SvelteNode[] }][]} */
const references = []; const references = [];
/** @type {[Scope, import('estree').Pattern | import('estree').MemberExpression][]} */ /** @type {[Scope, import('estree').Pattern | import('estree').MemberExpression][]} */
@ -265,7 +262,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat
const scope = state.scope.child(true); const scope = state.scope.child(true);
scopes.set(node, scope); scopes.set(node, scope);
next({ ...state, scope }); next({ scope });
}; };
/** /**
@ -274,7 +271,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat
const SvelteFragment = (node, { state, next }) => { const SvelteFragment = (node, { state, next }) => {
const scope = analyze_let_directives(node, state.scope); const scope = analyze_let_directives(node, state.scope);
scopes.set(node, scope); scopes.set(node, scope);
next({ ...state, scope }); next({ scope });
}; };
/** /**
@ -320,10 +317,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat
Identifier(node, { path, state }) { Identifier(node, { path, state }) {
const parent = path.at(-1); const parent = path.at(-1);
if (parent && is_reference(node, /** @type {import('estree').Node} */ (parent))) { if (parent && is_reference(node, /** @type {import('estree').Node} */ (parent))) {
references.push([ references.push([state.scope, { node, path: path.slice() }]);
state.scope,
{ node, path: path.slice(), is_template: state.is_template }
]);
} }
}, },
@ -346,7 +340,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat
} }
} }
next({ ...state, scope }); next({ scope });
}, },
SvelteFragment, SvelteFragment,
@ -354,7 +348,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat
RegularElement: SvelteFragment, RegularElement: SvelteFragment,
Component(node, { state, visit, path }) { Component(node, { state, visit, path }) {
state.scope.reference(b.id(node.name), path, false); state.scope.reference(b.id(node.name), path);
// let:x from the default slot is a weird one: // let:x from the default slot is a weird one:
// Its scope only applies to children that are not slots themselves. // Its scope only applies to children that are not slots themselves.
@ -378,7 +372,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat
} else if (child.type === 'SnippetBlock') { } else if (child.type === 'SnippetBlock') {
visit(child); visit(child);
} else { } else {
visit(child, { ...state, scope }); visit(child, { scope });
} }
} }
}, },
@ -412,7 +406,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat
if (node.id) scope.declare(node.id, 'normal', 'function'); if (node.id) scope.declare(node.id, 'normal', 'function');
add_params(scope, node.params); add_params(scope, node.params);
next({ scope, is_template: false }); next({ scope });
}, },
FunctionDeclaration(node, { state, next }) { FunctionDeclaration(node, { state, next }) {
@ -422,7 +416,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat
scopes.set(node, scope); scopes.set(node, scope);
add_params(scope, node.params); add_params(scope, node.params);
next({ scope, is_template: false }); next({ scope });
}, },
ArrowFunctionExpression(node, { state, next }) { ArrowFunctionExpression(node, { state, next }) {
@ -430,7 +424,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat
scopes.set(node, scope); scopes.set(node, scope);
add_params(scope, node.params); add_params(scope, node.params);
next({ scope, is_template: false }); next({ scope });
}, },
ForStatement: create_block_scope, ForStatement: create_block_scope,
@ -475,7 +469,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat
state.scope.declare(id, 'normal', 'let'); state.scope.declare(id, 'normal', 'let');
} }
next({ ...state, scope }); next({ scope });
} else { } else {
next(); next();
} }
@ -513,13 +507,13 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat
(node.key.type !== 'Identifier' || !node.index || node.key.name !== node.index); (node.key.type !== 'Identifier' || !node.index || node.key.name !== node.index);
scope.declare(b.id(node.index), is_keyed ? 'derived' : 'normal', 'const'); scope.declare(b.id(node.index), is_keyed ? 'derived' : 'normal', 'const');
} }
if (node.key) visit(node.key, { ...state, scope }); if (node.key) visit(node.key, { scope });
// children // children
for (const child of node.body.nodes) { for (const child of node.body.nodes) {
visit(child, { ...state, scope }); visit(child, { scope });
} }
if (node.fallback) visit(node.fallback, { ...state, scope }); if (node.fallback) visit(node.fallback, { scope });
// Check if inner scope shadows something from outer scope. // Check if inner scope shadows something from outer scope.
// This is necessary because we need access to the array expression of the each block // This is necessary because we need access to the array expression of the each block
@ -586,13 +580,13 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat
} }
} }
context.next({ ...state, scope: child_scope }); context.next({ scope: child_scope });
}, },
Fragment: (node, context) => { Fragment: (node, context) => {
const scope = context.state.scope.child(node.transparent); const scope = context.state.scope.child(node.transparent);
scopes.set(node, scope); scopes.set(node, scope);
context.next({ ...state, scope }); context.next({ scope });
}, },
BindDirective(node, context) { BindDirective(node, context) {
@ -629,8 +623,8 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat
// we do this after the fact, so that we don't need to worry // we do this after the fact, so that we don't need to worry
// about encountering references before their declarations // about encountering references before their declarations
for (const [scope, { node, path, is_template }] of references) { for (const [scope, { node, path }] of references) {
scope.reference(node, path, is_template); scope.reference(node, path);
} }
for (const [scope, node] of updates) { for (const [scope, node] of updates) {

@ -1,6 +1,6 @@
[ [
{ {
"code": "state-rune-not-mutated", "code": "state-not-mutated",
"end": { "end": {
"column": 11, "column": 11,
"line": 3 "line": 3

Loading…
Cancel
Save