From 5eff68f63df8fd2ccc3b0e591e47a43da4f8ccb3 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Fri, 5 Jul 2024 23:32:44 +0200 Subject: [PATCH] chore: runtime linting (#12314) - check that the runtime doesn't use methods that are too new - add linting rule to prevent references to the compiler in the runtime (this is important for the first check, else the ambient node typings would be included, which includes a definition for `at()`, which means we no longer would get errors when violating the "don't use new methods" rule in the runtime) - fix code as a result of these new checks closes #10438 --- eslint.config.js | 42 ++++++++++- packages/svelte/package.json | 2 +- .../src/compiler/phases/1-parse/index.js | 2 +- .../compiler/phases/1-parse/state/element.js | 2 +- .../compiler/phases/1-parse/utils/names.js | 74 ------------------ .../phases/3-transform/client/types.d.ts | 6 +- .../3-transform/client/visitors/template.js | 6 +- packages/svelte/src/constants.js | 75 +++++++++++++++++++ .../src/internal/client/dev/elements.js | 2 +- .../svelte/src/internal/client/runtime.js | 3 +- .../svelte/src/internal/shared/types.d.ts | 4 + .../svelte/src/internal/shared/validate.js | 2 +- packages/svelte/tsconfig.runtime.json | 15 ++++ 13 files changed, 146 insertions(+), 89 deletions(-) delete mode 100644 packages/svelte/src/compiler/phases/1-parse/utils/names.js create mode 100644 packages/svelte/tsconfig.runtime.json diff --git a/eslint.config.js b/eslint.config.js index d8875d37f9..60a1bb7e48 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -1,6 +1,38 @@ import svelte_config from '@sveltejs/eslint-config'; import lube from 'eslint-plugin-lube'; +const no_compiler_imports = { + meta: { + type: /** @type {const} */ ('problem'), + docs: { + description: + 'Enforce that there are no imports to the compiler in runtime code. ' + + 'This prevent accidental inclusion of the compiler runtime and ' + + "ensures that TypeScript does not pick up more ambient types (for example from Node) that shouldn't be available in the browser." + } + }, + create(context) { + return { + Program: () => { + // Do a simple string search because ESLint doesn't provide a way to check JSDoc comments. + // The string search could in theory yield false positives, but in practice it's unlikely. + const text = context.sourceCode.getText(); + const idx = Math.max(text.indexOf('../compiler/'), text.indexOf('#compiler')); + if (idx !== -1) { + context.report({ + loc: { + start: context.sourceCode.getLocFromIndex(idx), + end: context.sourceCode.getLocFromIndex(idx + 12) + }, + message: + 'References to compiler code are forbidden in runtime code (both for type and value imports)' + }); + } + } + }; + } +}; + /** @type {import('eslint').Linter.FlatConfig[]} */ export default [ ...svelte_config, @@ -11,7 +43,8 @@ export default [ } }, plugins: { - lube + lube, + custom: { rules: { no_compiler_imports } } }, rules: { '@typescript-eslint/await-thenable': 'error', @@ -37,6 +70,13 @@ export default [ 'no-console': 'off' } }, + { + files: ['packages/svelte/src/**/*'], + ignores: ['packages/svelte/src/compiler/**/*'], + rules: { + 'custom/no_compiler_imports': 'error' + } + }, { ignores: [ '**/*.d.ts', diff --git a/packages/svelte/package.json b/packages/svelte/package.json index 6774c1e5b0..2df9ed138b 100644 --- a/packages/svelte/package.json +++ b/packages/svelte/package.json @@ -106,7 +106,7 @@ "scripts": { "build": "node scripts/process-messages && rollup -c && pnpm generate:types && node scripts/check-treeshakeability.js", "dev": "node scripts/process-messages && rollup -cw", - "check": "tsc && cd ./tests/types && tsc", + "check": "tsc --project tsconfig.runtime.json && tsc && cd ./tests/types && tsc", "check:watch": "tsc --watch", "generate:version": "node ./scripts/generate-version.js", "generate:types": "node ./scripts/generate-types.js && tsc -p tsconfig.generated.json", diff --git a/packages/svelte/src/compiler/phases/1-parse/index.js b/packages/svelte/src/compiler/phases/1-parse/index.js index a6b407d44e..c5808f313a 100644 --- a/packages/svelte/src/compiler/phases/1-parse/index.js +++ b/packages/svelte/src/compiler/phases/1-parse/index.js @@ -3,7 +3,7 @@ import { isIdentifierStart, isIdentifierChar } from 'acorn'; import fragment from './state/fragment.js'; import { regex_whitespace } from '../patterns.js'; -import { reserved } from './utils/names.js'; +import { reserved } from '../../../constants.js'; import full_char_code_at from './utils/full_char_code_at.js'; import * as e from '../../errors.js'; import { create_fragment } from './utils/create.js'; diff --git a/packages/svelte/src/compiler/phases/1-parse/state/element.js b/packages/svelte/src/compiler/phases/1-parse/state/element.js index 2cc511411e..dcb7b23723 100644 --- a/packages/svelte/src/compiler/phases/1-parse/state/element.js +++ b/packages/svelte/src/compiler/phases/1-parse/state/element.js @@ -1,6 +1,6 @@ /** @import { Parser } from '../index.js' */ /** @import * as Compiler from '#compiler' */ -import { is_void } from '../utils/names.js'; +import { is_void } from '../../../../constants.js'; import read_expression from '../read/expression.js'; import { read_script } from '../read/script.js'; import read_style from '../read/style.js'; diff --git a/packages/svelte/src/compiler/phases/1-parse/utils/names.js b/packages/svelte/src/compiler/phases/1-parse/utils/names.js deleted file mode 100644 index 26dd20e40b..0000000000 --- a/packages/svelte/src/compiler/phases/1-parse/utils/names.js +++ /dev/null @@ -1,74 +0,0 @@ -export const reserved = [ - 'arguments', - 'await', - 'break', - 'case', - 'catch', - 'class', - 'const', - 'continue', - 'debugger', - 'default', - 'delete', - 'do', - 'else', - 'enum', - 'eval', - 'export', - 'extends', - 'false', - 'finally', - 'for', - 'function', - 'if', - 'implements', - 'import', - 'in', - 'instanceof', - 'interface', - 'let', - 'new', - 'null', - 'package', - 'private', - 'protected', - 'public', - 'return', - 'static', - 'super', - 'switch', - 'this', - 'throw', - 'true', - 'try', - 'typeof', - 'var', - 'void', - 'while', - 'with', - 'yield' -]; - -const void_element_names = [ - 'area', - 'base', - 'br', - 'col', - 'command', - 'embed', - 'hr', - 'img', - 'input', - 'keygen', - 'link', - 'meta', - 'param', - 'source', - 'track', - 'wbr' -]; - -/** @param {string} name */ -export function is_void(name) { - return void_element_names.includes(name) || name.toLowerCase() === '!doctype'; -} diff --git a/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts b/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts index aec2edb68d..3dbf5dc292 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts +++ b/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts @@ -8,7 +8,7 @@ import type { import type { Namespace, SvelteNode, ValidatedCompileOptions } from '#compiler'; import type { TransformState } from '../types.js'; import type { ComponentAnalysis } from '../../types.js'; -import type { Location } from 'locate-character'; +import type { SourceLocation } from '#shared'; export interface ClientTransformState extends TransformState { readonly private_state: Map; @@ -24,10 +24,6 @@ export interface ClientTransformState extends TransformState { readonly legacy_reactive_statements: Map; } -export type SourceLocation = - | [line: number, column: number] - | [line: number, column: number, SourceLocation[]]; - export interface ComponentClientTransformState extends ClientTransformState { readonly analysis: ComponentAnalysis; readonly options: ValidatedCompileOptions; diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index c879183d31..eb342613e4 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js @@ -1051,7 +1051,7 @@ function serialize_bind_this(bind_this, context, node) { } /** - * @param {import('../types.js').SourceLocation[]} locations + * @param {import('#shared').SourceLocation[]} locations */ function serialize_locations(locations) { return b.array( @@ -1905,7 +1905,7 @@ export const template_visitors = { state.init.push(b.stmt(b.call('$.transition', ...args))); }, RegularElement(node, context) { - /** @type {import('../types.js').SourceLocation} */ + /** @type {import('#shared').SourceLocation} */ let location = [-1, -1]; if (context.state.options.dev) { @@ -2133,7 +2133,7 @@ export const template_visitors = { context.state.template.push('>'); - /** @type {import('../types.js').SourceLocation[]} */ + /** @type {import('#shared').SourceLocation[]} */ const child_locations = []; /** @type {import('../types').ComponentClientTransformState} */ diff --git a/packages/svelte/src/constants.js b/packages/svelte/src/constants.js index a8963d6854..619a83562d 100644 --- a/packages/svelte/src/constants.js +++ b/packages/svelte/src/constants.js @@ -293,3 +293,78 @@ export function is_capture_event(name, mode = 'exclude-on') { ? name !== 'gotpointercapture' && name !== 'lostpointercapture' : name !== 'ongotpointercapture' && name !== 'onlostpointercapture'; } + +export const reserved = [ + 'arguments', + 'await', + 'break', + 'case', + 'catch', + 'class', + 'const', + 'continue', + 'debugger', + 'default', + 'delete', + 'do', + 'else', + 'enum', + 'eval', + 'export', + 'extends', + 'false', + 'finally', + 'for', + 'function', + 'if', + 'implements', + 'import', + 'in', + 'instanceof', + 'interface', + 'let', + 'new', + 'null', + 'package', + 'private', + 'protected', + 'public', + 'return', + 'static', + 'super', + 'switch', + 'this', + 'throw', + 'true', + 'try', + 'typeof', + 'var', + 'void', + 'while', + 'with', + 'yield' +]; + +const void_element_names = [ + 'area', + 'base', + 'br', + 'col', + 'command', + 'embed', + 'hr', + 'img', + 'input', + 'keygen', + 'link', + 'meta', + 'param', + 'source', + 'track', + 'wbr' +]; + +/** @param {string} name */ +export function is_void(name) { + return void_element_names.includes(name) || name.toLowerCase() === '!doctype'; +} diff --git a/packages/svelte/src/internal/client/dev/elements.js b/packages/svelte/src/internal/client/dev/elements.js index 9b921a9f08..14713736ad 100644 --- a/packages/svelte/src/internal/client/dev/elements.js +++ b/packages/svelte/src/internal/client/dev/elements.js @@ -1,4 +1,4 @@ -/** @import { SourceLocation } from '../../../compiler/phases/3-transform/client/types.js' */ +/** @import { SourceLocation } from '#shared' */ import { HYDRATION_END, HYDRATION_START } from '../../../constants.js'; import { hydrating } from '../dom/hydration.js'; diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 07462c5244..e699183d02 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -224,10 +224,11 @@ function handle_error(error, effect, component_context) { let current_context = component_context; while (current_context !== null) { + /** @type {string} */ var filename = current_context.function?.filename; if (filename) { - const file = filename.split('/').at(-1); + const file = filename.split('/').pop(); component_stack.push(file); } diff --git a/packages/svelte/src/internal/shared/types.d.ts b/packages/svelte/src/internal/shared/types.d.ts index ffde04249d..426d928ce3 100644 --- a/packages/svelte/src/internal/shared/types.d.ts +++ b/packages/svelte/src/internal/shared/types.d.ts @@ -2,3 +2,7 @@ export type Store = { subscribe: (run: (value: V) => void) => () => void; set(value: V): void; }; + +export type SourceLocation = + | [line: number, column: number] + | [line: number, column: number, SourceLocation[]]; diff --git a/packages/svelte/src/internal/shared/validate.js b/packages/svelte/src/internal/shared/validate.js index 3517d32c2c..399ace8b58 100644 --- a/packages/svelte/src/internal/shared/validate.js +++ b/packages/svelte/src/internal/shared/validate.js @@ -1,4 +1,4 @@ -import { is_void } from '../../compiler/phases/1-parse/utils/names.js'; +import { is_void } from '../../constants.js'; import * as w from './warnings.js'; import * as e from './errors.js'; diff --git a/packages/svelte/tsconfig.runtime.json b/packages/svelte/tsconfig.runtime.json new file mode 100644 index 0000000000..09c49764bd --- /dev/null +++ b/packages/svelte/tsconfig.runtime.json @@ -0,0 +1,15 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + // Ensure we don't use any methods that are not available in all browser for a long period of time + // so that people don't need to polyfill them. Example array.at(...) was introduced to Safari only in 2022 + "target": "es2021", + "lib": ["es2021", "DOM", "DOM.Iterable"], + "types": [] // prevent automatic inclusion of @types/node + }, + "include": ["./src/"], + // Compiler is allowed to use more recent methods; people using it in the browser are expected to know + // how to polyfill missing methods. Also make sure to not include test files as these include Vitest + // which then loads node globals. + "exclude": ["./src/compiler/**/*", "./src/**/*.test.ts"] +}