Skip to content

fix: strip typescript assertions before analysis #10329

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

Merged
merged 8 commits into from
Jan 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/sixty-items-crash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"svelte": patch
---

fix: bindings with typescript assertions
23 changes: 17 additions & 6 deletions packages/svelte/src/compiler/index.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { parse as _parse } from './phases/1-parse/index.js';
import { getLocator } from 'locate-character';
import { walk } from 'zimmerframe';
import { CompileError } from './errors.js';
import { convert } from './legacy.js';
import { parse as parse_acorn } from './phases/1-parse/acorn.js';
import { parse as _parse } from './phases/1-parse/index.js';
import { remove_typescript_nodes } from './phases/1-parse/remove_typescript_nodes.js';
import { analyze_component, analyze_module } from './phases/2-analyze/index.js';
import { transform_component, transform_module } from './phases/3-transform/index.js';
import { getLocator } from 'locate-character';
import { walk } from 'zimmerframe';
import { validate_component_options, validate_module_options } from './validate-options.js';
import { convert } from './legacy.js';
import { CompileError } from './errors.js';
export { default as preprocess } from './preprocess/index.js';

/**
Expand All @@ -20,14 +21,24 @@ export { default as preprocess } from './preprocess/index.js';
export function compile(source, options) {
try {
const validated = validate_component_options(options, '');
const parsed = _parse(source);
let parsed = _parse(source);

const combined_options = /** @type {import('#compiler').ValidatedCompileOptions} */ ({
...validated,
...parsed.options
});

if (parsed.metadata.ts) {
parsed = {
...parsed,
fragment: parsed.fragment && remove_typescript_nodes(parsed.fragment),
instance: parsed.instance && remove_typescript_nodes(parsed.instance),
module: parsed.module && remove_typescript_nodes(parsed.module)
};
}

const analysis = analyze_component(parsed, combined_options);

const result = transform_component(analysis, source, combined_options);
return result;
} catch (e) {
Expand Down
5 changes: 4 additions & 1 deletion packages/svelte/src/compiler/phases/1-parse/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,10 @@ export class Parser {
end: null,
type: 'Root',
fragment: create_fragment(),
options: null
options: null,
metadata: {
ts: this.ts
}
};

this.stack.push(this.root);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { walk } from 'zimmerframe';
import * as b from '../../utils/builders.js';

/** @type {import('zimmerframe').Visitors<any, any>} */
export const remove_types = {
/** @type {import('zimmerframe').Visitors<any, null>} */
const visitors = {
ImportDeclaration(node) {
if (node.importKind === 'type') return b.empty;

Expand Down Expand Up @@ -54,3 +55,12 @@ export const remove_types = {
return b.empty;
}
};

/**
* @template T
* @param {T} ast
* @returns {T}
*/
export function remove_typescript_nodes(ast) {
return walk(ast, null, visitors);
}
5 changes: 2 additions & 3 deletions packages/svelte/src/compiler/phases/2-analyze/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ import {
extract_paths,
is_event_attribute,
is_text_attribute,
object,
unwrap_ts_expression
object
} from '../../utils/ast.js';
import * as b from '../../utils/builders.js';
import { ReservedKeywords, Runes, SVGElements } from '../constants.js';
Expand Down Expand Up @@ -706,7 +705,7 @@ const runes_scope_tweaker = {
}
},
VariableDeclarator(node, { state }) {
const init = unwrap_ts_expression(node.init);
const init = node.init;
if (!init || init.type !== 'CallExpression') return;
const rune = get_rune(init, state.scope);
if (rune === null) return;
Expand Down
13 changes: 4 additions & 9 deletions packages/svelte/src/compiler/phases/2-analyze/validation.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
import { error } from '../../errors.js';
import {
extract_identifiers,
get_parent,
is_text_attribute,
unwrap_ts_expression
} from '../../utils/ast.js';
import { extract_identifiers, get_parent, is_text_attribute } from '../../utils/ast.js';
import { warn } from '../../warnings.js';
import fuzzymatch from '../1-parse/utils/fuzzymatch.js';
import { disallowed_parapgraph_contents, interactive_elements } from '../1-parse/utils/html.js';
Expand Down Expand Up @@ -338,11 +333,11 @@ const validation = {
BindDirective(node, context) {
validate_no_const_assignment(node, node.expression, context.state.scope, true);

const assignee = unwrap_ts_expression(node.expression);
const assignee = node.expression;
let left = assignee;

while (left.type === 'MemberExpression') {
left = unwrap_ts_expression(/** @type {import('estree').MemberExpression} */ (left.object));
left = /** @type {import('estree').MemberExpression} */ (left.object);
}

if (left.type !== 'Identifier') {
Expand Down Expand Up @@ -950,7 +945,7 @@ export const validation_runes = merge(validation, a11y_validators, {
next({ ...state });
},
VariableDeclarator(node, { state, path }) {
const init = unwrap_ts_expression(node.init);
const init = node.init;
const rune = get_rune(init, state.scope);

if (rune === null) return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { javascript_visitors } from './visitors/javascript.js';
import { javascript_visitors_runes } from './visitors/javascript-runes.js';
import { javascript_visitors_legacy } from './visitors/javascript-legacy.js';
import { is_state_source, serialize_get_binding } from './utils.js';
import { remove_types } from '../typescript.js';

/**
* This function ensures visitor sets don't accidentally clobber each other
Expand Down Expand Up @@ -102,7 +101,6 @@ export function client_component(source, analysis, options) {
state,
combine_visitors(
set_scope(analysis.module.scopes),
remove_types,
global_visitors,
// @ts-expect-error TODO
javascript_visitors,
Expand All @@ -118,21 +116,18 @@ export function client_component(source, analysis, options) {
instance_state,
combine_visitors(
set_scope(analysis.instance.scopes),
{ ...remove_types, ImportDeclaration: undefined, ExportNamedDeclaration: undefined },
global_visitors,
// @ts-expect-error TODO
javascript_visitors,
analysis.runes ? javascript_visitors_runes : javascript_visitors_legacy,
{
ImportDeclaration(node, context) {
// @ts-expect-error
state.hoisted.push(remove_types.ImportDeclaration(node, context));
ImportDeclaration(node) {
state.hoisted.push(node);
return b.empty;
},
ExportNamedDeclaration(node, context) {
if (node.declaration) {
// @ts-expect-error
return remove_types.ExportNamedDeclaration(context.visit(node.declaration), context);
return context.visit(node.declaration);
}

return b.empty;
Expand All @@ -148,7 +143,6 @@ export function client_component(source, analysis, options) {
{ ...state, scope: analysis.instance.scope },
combine_visitors(
set_scope(analysis.template.scopes),
remove_types,
global_visitors,
// @ts-expect-error TODO
template_visitors
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
import * as b from '../../../utils/builders.js';
import {
extract_paths,
is_simple_expression,
object,
unwrap_ts_expression
} from '../../../utils/ast.js';
import { extract_paths, is_simple_expression, object } from '../../../utils/ast.js';
import { error } from '../../../errors.js';
import {
PROPS_IS_LAZY_INITIAL,
Expand Down Expand Up @@ -228,7 +223,7 @@ function is_expression_async(expression) {
export function serialize_set_binding(node, context, fallback, options) {
const { state, visit } = context;

const assignee = unwrap_ts_expression(node.left);
const assignee = node.left;
if (
assignee.type === 'ArrayPattern' ||
assignee.type === 'ObjectPattern' ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { is_hoistable_function, transform_inspect_rune } from '../../utils.js';
import * as b from '../../../../utils/builders.js';
import * as assert from '../../../../utils/assert.js';
import { get_prop_source, is_state_source, should_proxy_or_freeze } from '../utils.js';
import { extract_paths, unwrap_ts_expression } from '../../../../utils/ast.js';
import { extract_paths } from '../../../../utils/ast.js';

/** @type {import('../types.js').ComponentVisitors} */
export const javascript_visitors_runes = {
Expand Down Expand Up @@ -174,7 +174,7 @@ export const javascript_visitors_runes = {
const declarations = [];

for (const declarator of node.declarations) {
const init = unwrap_ts_expression(declarator.init);
const init = declarator.init;
const rune = get_rune(init, state.scope);
if (!rune || rune === '$effect.active' || rune === '$effect.root' || rune === '$inspect') {
if (init != null && is_hoistable_function(init)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ import {
extract_paths,
is_event_attribute,
is_text_attribute,
object,
unwrap_ts_expression
object
} from '../../../../utils/ast.js';
import { binding_properties } from '../../../bindings.js';
import {
Expand Down Expand Up @@ -2576,7 +2575,7 @@ export const template_visitors = {
},
BindDirective(node, context) {
const { state, path, visit } = context;
const expression = unwrap_ts_expression(node.expression);
const expression = node.expression;
const getter = b.thunk(/** @type {import('estree').Expression} */ (visit(expression)));
const assignment = b.assignment('=', expression, b.id('$$value'));
const setter = b.arrow(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
import { walk } from 'zimmerframe';
import { set_scope, get_rune } from '../../scope.js';
import {
extract_identifiers,
extract_paths,
is_event_attribute,
unwrap_ts_expression
} from '../../../utils/ast.js';
import { extract_identifiers, extract_paths, is_event_attribute } from '../../../utils/ast.js';
import * as b from '../../../utils/builders.js';
import is_reference from 'is-reference';
import {
Expand All @@ -24,7 +19,6 @@ import { create_attribute, is_custom_element_node, is_element_node } from '../..
import { error } from '../../../errors.js';
import { binding_properties } from '../../bindings.js';
import { regex_starts_with_newline, regex_whitespaces_strict } from '../../patterns.js';
import { remove_types } from '../typescript.js';
import { DOMBooleanAttributes } from '../../../../constants.js';
import { sanitize_template_string } from '../../../utils/sanitize_template_string.js';

Expand Down Expand Up @@ -570,7 +564,7 @@ const javascript_visitors_runes = {
const declarations = [];

for (const declarator of node.declarations) {
const init = unwrap_ts_expression(declarator.init);
const init = declarator.init;
const rune = get_rune(init, state.scope);
if (!rune || rune === '$effect.active' || rune === '$inspect') {
declarations.push(/** @type {import('estree').VariableDeclarator} */ (visit(declarator)));
Expand Down Expand Up @@ -1963,34 +1957,36 @@ export function server_component(analysis, options) {
};

const module = /** @type {import('estree').Program} */ (
walk(/** @type {import('#compiler').SvelteNode} */ (analysis.module.ast), state, {
...set_scope(analysis.module.scopes),
...global_visitors,
...remove_types,
...javascript_visitors,
...(analysis.runes ? javascript_visitors_runes : javascript_visitors_legacy)
})
walk(
/** @type {import('#compiler').SvelteNode} */ (analysis.module.ast),
state,
// @ts-expect-error TODO: zimmerframe types
{
...set_scope(analysis.module.scopes),
...global_visitors,
...javascript_visitors,
...(analysis.runes ? javascript_visitors_runes : javascript_visitors_legacy)
}
)
);

const instance = /** @type {import('estree').Program} */ (
walk(
/** @type {import('#compiler').SvelteNode} */ (analysis.instance.ast),
{ ...state, scope: analysis.instance.scope },
// @ts-expect-error TODO: zimmerframe types
{
...set_scope(analysis.instance.scopes),
...global_visitors,
...{ ...remove_types, ImportDeclaration: undefined, ExportNamedDeclaration: undefined },
...javascript_visitors,
...(analysis.runes ? javascript_visitors_runes : javascript_visitors_legacy),
ImportDeclaration(node, context) {
// @ts-expect-error
state.hoisted.push(remove_types.ImportDeclaration(node, context));
ImportDeclaration(node) {
state.hoisted.push(node);
return b.empty;
},
ExportNamedDeclaration(node, context) {
if (node.declaration) {
// @ts-expect-error
return remove_types.ExportNamedDeclaration(context.visit(node.declaration), context);
return context.visit(node.declaration);
}

return b.empty;
Expand All @@ -2003,10 +1999,10 @@ export function server_component(analysis, options) {
walk(
/** @type {import('#compiler').SvelteNode} */ (analysis.template.ast),
{ ...state, scope: analysis.template.scope },
// @ts-expect-error TODO: zimmerframe types
{
...set_scope(analysis.template.scopes),
...global_visitors,
...remove_types,
...template_visitors
}
)
Expand Down
4 changes: 4 additions & 0 deletions packages/svelte/src/compiler/types/template.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ export interface Root extends BaseNode {
instance: Script | null;
/** The parsed `<script context="module">` element, if exists */
module: Script | null;
metadata: {
/** Whether the component was parsed with typescript */
ts: boolean;
};
}

export interface SvelteOptions {
Expand Down
Loading