From 10a66d0968ee2f1e0bcdab499c5fc49e44a35705 Mon Sep 17 00:00:00 2001 From: yosuke ota Date: Fri, 2 May 2025 15:01:35 +0900 Subject: [PATCH 01/10] feat: add `svelte/no-top-level-browser-globals` rule --- README.md | 1 + docs/rules.md | 1 + docs/rules/no-top-level-browser-globals.md | 56 +++ packages/eslint-plugin-svelte/package.json | 3 +- .../eslint-plugin-svelte/src/rule-types.ts | 5 + .../src/rules/no-goto-without-base.ts | 2 +- .../src/rules/no-navigation-without-base.ts | 6 +- .../src/rules/no-top-level-browser-globals.ts | 397 ++++++++++++++++++ .../@eslint-community/eslint-utils.d.ts | 29 +- .../eslint-plugin-svelte/src/utils/rules.ts | 2 + .../invalid/test01-errors.yaml | 4 + .../invalid/test01-input.svelte | 4 + .../valid/effect01-input.svelte | 6 + .../valid/env01-input.svelte | 8 + .../valid/env02-input.svelte | 8 + .../valid/on-mount01-input.svelte | 8 + .../src/rules/no-top-level-browser-globals.ts | 16 + 17 files changed, 540 insertions(+), 16 deletions(-) create mode 100644 docs/rules/no-top-level-browser-globals.md create mode 100644 packages/eslint-plugin-svelte/src/rules/no-top-level-browser-globals.ts create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/test01-errors.yaml create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/test01-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/effect01-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/env01-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/env02-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/on-mount01-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/src/rules/no-top-level-browser-globals.ts diff --git a/README.md b/README.md index 8bc75adca..9cccc33bd 100644 --- a/README.md +++ b/README.md @@ -271,6 +271,7 @@ These rules relate to possible syntax or logic errors in Svelte code: | [svelte/no-reactive-reassign](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-reactive-reassign/) | disallow reassigning reactive values | :star: | | [svelte/no-shorthand-style-property-overrides](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-shorthand-style-property-overrides/) | disallow shorthand style properties that override related longhand properties | :star: | | [svelte/no-store-async](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-store-async/) | disallow using async/await inside svelte stores because it causes issues with the auto-unsubscribing features | :star: | +| [svelte/no-top-level-browser-globals](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-top-level-browser-globals/) | disallow using top-level browser global variables | | | [svelte/no-unknown-style-directive-property](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unknown-style-directive-property/) | disallow unknown `style:property` | :star: | | [svelte/require-store-callbacks-use-set-param](https://sveltejs.github.io/eslint-plugin-svelte/rules/require-store-callbacks-use-set-param/) | store callbacks must use `set` param | :bulb: | | [svelte/require-store-reactive-access](https://sveltejs.github.io/eslint-plugin-svelte/rules/require-store-reactive-access/) | disallow to use of the store itself as an operand. Need to use $ prefix or get function. | :star::wrench: | diff --git a/docs/rules.md b/docs/rules.md index 1fdd627c8..459eb2ae4 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -28,6 +28,7 @@ These rules relate to possible syntax or logic errors in Svelte code: | [svelte/no-reactive-reassign](./rules/no-reactive-reassign.md) | disallow reassigning reactive values | :star: | | [svelte/no-shorthand-style-property-overrides](./rules/no-shorthand-style-property-overrides.md) | disallow shorthand style properties that override related longhand properties | :star: | | [svelte/no-store-async](./rules/no-store-async.md) | disallow using async/await inside svelte stores because it causes issues with the auto-unsubscribing features | :star: | +| [svelte/no-top-level-browser-globals](./rules/no-top-level-browser-globals.md) | disallow using top-level browser global variables | | | [svelte/no-unknown-style-directive-property](./rules/no-unknown-style-directive-property.md) | disallow unknown `style:property` | :star: | | [svelte/require-store-callbacks-use-set-param](./rules/require-store-callbacks-use-set-param.md) | store callbacks must use `set` param | :bulb: | | [svelte/require-store-reactive-access](./rules/require-store-reactive-access.md) | disallow to use of the store itself as an operand. Need to use $ prefix or get function. | :star::wrench: | diff --git a/docs/rules/no-top-level-browser-globals.md b/docs/rules/no-top-level-browser-globals.md new file mode 100644 index 000000000..d7f6e4966 --- /dev/null +++ b/docs/rules/no-top-level-browser-globals.md @@ -0,0 +1,56 @@ +--- +pageClass: 'rule-details' +sidebarDepth: 0 +title: 'svelte/no-top-level-browser-globals' +description: 'disallow using top-level browser global variables' +--- + +# svelte/no-top-level-browser-globals + +> disallow using top-level browser global variables + +- :exclamation: **_This rule has not been released yet._** + +## :book: Rule Details + +This rule reports top-level browser global variables in Svelte components. +This rule helps prevent the use of browser global variables that can cause errors in SSR (Server Side Rendering). + + + +```svelte + +``` + +## :wrench: Options + +Nothing. + +## :books: Further Reading + +- [`$app/environment` documentation > browser](https://svelte.dev/docs/kit/$app-environment#browser) + +## :mag: Implementation + +- [Rule source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/packages/eslint-plugin-svelte/src/rules/no-top-level-browser-globals.ts) +- [Test source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/packages/eslint-plugin-svelte/tests/src/rules/no-top-level-browser-globals.ts) diff --git a/packages/eslint-plugin-svelte/package.json b/packages/eslint-plugin-svelte/package.json index 14759b1e1..245c10796 100644 --- a/packages/eslint-plugin-svelte/package.json +++ b/packages/eslint-plugin-svelte/package.json @@ -56,9 +56,10 @@ } }, "dependencies": { - "@eslint-community/eslint-utils": "^4.4.1", + "@eslint-community/eslint-utils": "^4.6.1", "@jridgewell/sourcemap-codec": "^1.5.0", "esutils": "^2.0.3", + "globals": "^16.0.0", "known-css-properties": "^0.36.0", "postcss": "^8.4.49", "postcss-load-config": "^3.1.4", diff --git a/packages/eslint-plugin-svelte/src/rule-types.ts b/packages/eslint-plugin-svelte/src/rule-types.ts index 8c56fd802..2fd9c7db2 100644 --- a/packages/eslint-plugin-svelte/src/rule-types.ts +++ b/packages/eslint-plugin-svelte/src/rule-types.ts @@ -251,6 +251,11 @@ export interface RuleOptions { * @see https://sveltejs.github.io/eslint-plugin-svelte/rules/no-target-blank/ */ 'svelte/no-target-blank'?: Linter.RuleEntry + /** + * disallow using top-level browser global variables + * @see https://sveltejs.github.io/eslint-plugin-svelte/rules/no-top-level-browser-globals/ + */ + 'svelte/no-top-level-browser-globals'?: Linter.RuleEntry<[]> /** * disallow trailing whitespace at the end of lines * @see https://sveltejs.github.io/eslint-plugin-svelte/rules/no-trailing-spaces/ diff --git a/packages/eslint-plugin-svelte/src/rules/no-goto-without-base.ts b/packages/eslint-plugin-svelte/src/rules/no-goto-without-base.ts index 021bdb5b4..78182e0d0 100644 --- a/packages/eslint-plugin-svelte/src/rules/no-goto-without-base.ts +++ b/packages/eslint-plugin-svelte/src/rules/no-goto-without-base.ts @@ -106,7 +106,7 @@ function extractGotoReferences(referenceTracker: ReferenceTracker): TSESTree.Cal } } }), - ({ node }) => node + ({ node }) => node as TSESTree.CallExpression ); } diff --git a/packages/eslint-plugin-svelte/src/rules/no-navigation-without-base.ts b/packages/eslint-plugin-svelte/src/rules/no-navigation-without-base.ts index 2bebe6d16..1e19b9b99 100644 --- a/packages/eslint-plugin-svelte/src/rules/no-navigation-without-base.ts +++ b/packages/eslint-plugin-svelte/src/rules/no-navigation-without-base.ts @@ -171,13 +171,13 @@ function extractFunctionCallReferences(referenceTracker: ReferenceTracker): { return { goto: rawReferences .filter(({ path }) => path[path.length - 1] === 'goto') - .map(({ node }) => node), + .map(({ node }) => node as TSESTree.CallExpression), pushState: rawReferences .filter(({ path }) => path[path.length - 1] === 'pushState') - .map(({ node }) => node), + .map(({ node }) => node as TSESTree.CallExpression), replaceState: rawReferences .filter(({ path }) => path[path.length - 1] === 'replaceState') - .map(({ node }) => node) + .map(({ node }) => node as TSESTree.CallExpression) }; } diff --git a/packages/eslint-plugin-svelte/src/rules/no-top-level-browser-globals.ts b/packages/eslint-plugin-svelte/src/rules/no-top-level-browser-globals.ts new file mode 100644 index 000000000..ff643f300 --- /dev/null +++ b/packages/eslint-plugin-svelte/src/rules/no-top-level-browser-globals.ts @@ -0,0 +1,397 @@ +import type { TrackedReferences } from '@eslint-community/eslint-utils'; +import { ReferenceTracker, getStaticValue, getPropertyName } from '@eslint-community/eslint-utils'; +import { createRule } from '../utils/index.js'; +import globals from 'globals'; +import type { TSESTree } from '@typescript-eslint/types'; +import { findVariable, getScope } from '../utils/ast-utils.js'; + +export default createRule('no-top-level-browser-globals', { + meta: { + docs: { + description: 'disallow using top-level browser global variables', + category: 'Possible Errors', + recommended: false + }, + schema: [], + messages: { + unexpectedGlobal: 'Unexpected top-level browser global variable "{{name}}".' + }, + type: 'problem', + conditions: [] + }, + create(context) { + const sourceCode = context.sourceCode; + if (!sourceCode.parserServices.isSvelte && !sourceCode.parserServices.isSvelteScript) { + return {}; + } + const blowerGlobals = getBrowserGlobals(); + + const functions: TSESTree.FunctionLike[] = []; + + function isTopLevelLocation(node: TSESTree.Node) { + for (const func of functions) { + if (func.range[0] <= node.range[0] && node.range[1] <= func.range[1]) { + return false; + } + } + return true; + } + + function enterFunction(node: TSESTree.FunctionLike) { + functions.push(node); + } + + function verifyGlobalReferences() { + const referenceTracker = new ReferenceTracker(sourceCode.scopeManager.globalScope!, { + // Specifies the global variables that are allowed to prevent `window.window` from being iterated over. + globalObjectNames: ['globalThis'] + }); + + type MaybeGuard = { + referenceNode: TSESTree.Node; + isAvailableLocation: (node: TSESTree.Node) => boolean; + // The guard that checks whether the browser environment is set to true. + browserEnvironment: boolean; + used?: boolean; + }; + const maybeGuards: MaybeGuard[] = []; + + /** + * Checks whether the node is in a location where the expression is available or not. + * @returns `true` if the expression is available. + */ + function isAvailableLocation({ node }: { node: TSESTree.Node }) { + for (const guard of maybeGuards.reverse()) { + if (guard.browserEnvironment || equalNode(guard.referenceNode, node)) { + if (guard.isAvailableLocation(node)) { + guard.used = true; + return true; + } + } + } + return false; + } + + /** + * Iterate over the references of modules that can check the browser environment. + */ + function* iterateBrowserCheckerModuleReferences(): Iterable { + for (const ref of referenceTracker.iterateEsmReferences({ + 'esm-env': { + [ReferenceTracker.ESM]: true, + BROWSER: { + [ReferenceTracker.READ]: true + } + }, + '$app/environment': { + [ReferenceTracker.ESM]: true, + browser: { + [ReferenceTracker.READ]: true + } + } + })) { + if (ref.node.type === 'Identifier' || ref.node.type === 'MemberExpression') { + yield ref.node; + } else if (ref.node.type === 'ImportSpecifier') { + const variable = findVariable(context, ref.node.local); + if (variable) + for (const reference of variable.references) { + if (reference.isRead() && reference.identifier.type === 'Identifier') { + yield reference.identifier; + } + } + } + } + } + + // Collects guarded location checkers by checking module references + // that can check the browser environment. + for (const referenceNode of iterateBrowserCheckerModuleReferences()) { + if (!isTopLevelLocation(referenceNode)) continue; + const guardChecker = getGuardChecker({ node: referenceNode }); + if (guardChecker) { + maybeGuards.push({ + referenceNode, + isAvailableLocation: guardChecker, + browserEnvironment: true + }); + } + } + + const reportCandidates: TrackedReferences[] = []; + + // Collects references to global variables. + for (const ref of referenceTracker.iterateGlobalReferences({ + ...Object.fromEntries( + blowerGlobals.map((name) => [ + name, + { + [ReferenceTracker.READ]: true + } + ]) + ) + })) { + if (!isTopLevelLocation(ref.node)) continue; + const guardChecker = getGuardCheckerFromReference(ref.node); + if (guardChecker) { + const name = ref.path.join('.'); + maybeGuards.push({ + referenceNode: ref.node, + isAvailableLocation: guardChecker, + browserEnvironment: name === 'window' || name === 'document' + }); + } else { + reportCandidates.push(ref); + } + } + + for (const ref of reportCandidates) { + if (isAvailableLocation({ node: ref.node })) { + continue; + } + context.report({ + node: ref.node, + messageId: 'unexpectedGlobal', + data: { + name: ref.path.join('.') + } + }); + } + } + + return { + ':function': enterFunction, + 'Program:exit': verifyGlobalReferences + }; + + /** + * If the node is a reference used in a guard clause that checks if the node is in a browser environment, + * it returns information about the expression that checks if the browser variable is available. + * @returns The guard info. + */ + function getGuardCheckerFromReference( + node: TSESTree.Node + ): ((node: TSESTree.Node) => boolean) | null { + const parent = node.parent; + if (!parent) return null; + if (parent.type === 'BinaryExpression') { + if ( + parent.operator === 'instanceof' && + parent.left === node && + node.type === 'MemberExpression' + ) { + // e.g. if (globalThis.window instanceof X) + return getGuardChecker({ node: parent }); + } + const operand = + parent.left === node ? parent.right : parent.right === node ? parent.left : null; + if (!operand) return null; + + const staticValue = getStaticValue(operand, getScope(context, operand)); + if (!staticValue) { + return null; + } + if (staticValue.value === undefined && node.type === 'MemberExpression') { + if (parent.operator === '!==' || parent.operator === '!=') { + // e.g. if (globalThis.window !== undefined), if (globalThis.window != undefined) + return getGuardChecker({ node: parent }); + } else if (parent.operator === '===' || parent.operator === '==') { + // e.g. if (globalThis.window === undefined), if (globalThis.window == undefined) + return getGuardChecker({ node: parent, not: true }); + } + } else if (staticValue.value === null && node.type === 'MemberExpression') { + if (parent.operator === '!=') { + // e.g. if (globalThis.window != null) + return getGuardChecker({ node: parent }); + } else if (parent.operator === '==') { + // e.g. if (globalThis.window == null) + return getGuardChecker({ node: parent, not: true }); + } + } + return null; + } + if ( + parent.type === 'UnaryExpression' && + parent.operator === 'typeof' && + parent.argument === node + ) { + const pp = parent.parent; + if (!pp || pp.type !== 'BinaryExpression') { + return null; + } + const staticValue = getStaticValue( + pp.left === node ? pp.right : pp.left, + getScope(context, node) + ); + if (!staticValue) { + return null; + } + if (staticValue.value !== 'undefined' && staticValue.value !== 'object') { + return null; + } + if (pp.operator === '!==' || pp.operator === '!=') { + if (staticValue.value === 'undefined') { + // e.g. if (typeof window !== "undefined"), if (typeof window != "undefined") + return getGuardChecker({ node: pp }); + } + // e.g. if (typeof window !== "object"), if (typeof window != "object") + return getGuardChecker({ node: pp, not: true }); + } else if (pp.operator === '===' || pp.operator === '==') { + if (staticValue.value === 'undefined') { + // e.g. if (typeof window === "undefined"), if (typeof window == "undefined") + return getGuardChecker({ node: pp, not: true }); + } + // e.g. if (typeof window === "object"), if (typeof window == "object") + return getGuardChecker({ node: pp }); + } + return null; + } + + if (node.type === 'MemberExpression') { + // e.g. if (globalThis.window) + return getGuardChecker({ node }); + } + return null; + } + + /** + * If the node is a guard clause checking, + * returns a function to check if the node is available. + */ + function getGuardChecker(guardInfo: { + node: TSESTree.Expression; + not?: boolean; + }): ((node: TSESTree.Node) => boolean) | null { + const parent = guardInfo.node.parent; + if (!parent) { + return null; + } + if (parent.type === 'ConditionalExpression') { + const block = guardInfo.not ? parent.alternate : parent.consequent; + return (n) => block.range[0] <= n.range[0] && n.range[1] <= block.range[1]; + } + if (parent.type === 'UnaryExpression' && parent.operator === '!') { + return getGuardChecker({ not: !guardInfo.not, node: parent }); + } + if (parent.type === 'IfStatement' && parent.test === guardInfo.node) { + if (!guardInfo.not) { + const block = parent.consequent; + return (n) => block.range[0] <= n.range[0] && n.range[1] <= block.range[1]; + } + // e.g. if (typeof x === 'undefined') + if (parent.alternate) { + const block = parent.alternate; + return (n) => block.range[0] <= n.range[0] && n.range[1] <= block.range[1]; + } + if (!hasJumpStatementInAllPath(parent.consequent)) { + return null; + } + const pp = parent.parent; + if (!pp || (pp.type !== 'BlockStatement' && pp.type !== 'Program')) { + return null; + } + const start = parent.range[1]; + const end = pp.range[1]; + + return (n) => start <= n.range[0] && n.range[1] <= end; + } + return null; + } + + /** + * Checks whether or not the two given nodes are same. + * @param a A node 1 to compare. + * @param b A node 2 to compare. + */ + function equalNode(a: TSESTree.Node, b: TSESTree.Node) { + if (a.type === 'Identifier' && b.type === 'Identifier') { + const leftVar = findVariable(context, a); + const rightVar = findVariable(context, b); + return leftVar && rightVar && leftVar === rightVar; + } + if (a.type === 'MemberExpression' && b.type === 'MemberExpression') { + if (!equalNode(a.object, b.object)) { + return false; + } + const leftKey = getPropertyName(a); + const rightKey = getPropertyName(b); + return leftKey && rightKey && leftKey === rightKey; + } + if (isSkipExpression(a)) { + return equalNode(a.expression, b); + } + if (isSkipExpression(b)) { + return equalNode(a, b.expression); + } + return false; + } + } +}); + +/** + * Get the list of browser-specific globals. + */ +function getBrowserGlobals() { + const nodeGlobals = new Set(Object.keys(globals.node)); + return [ + 'window', + 'document', + ...Object.keys(globals.browser).filter((name) => !nodeGlobals.has(name)) + ]; +} + +/** + * Checks whether all paths of a given statement have jump statements. + * @param {Statement} statement + * @returns {boolean} + */ +function hasJumpStatementInAllPath(statement: TSESTree.Statement): boolean { + if (isJumpStatement(statement)) { + return true; + } + if (statement.type === 'BlockStatement') { + return statement.body.some(hasJumpStatementInAllPath); + } + if (statement.type === 'IfStatement') { + if (!statement.alternate) { + return false; + } + return ( + hasJumpStatementInAllPath(statement.alternate) && + hasJumpStatementInAllPath(statement.consequent) + ); + } + return false; +} + +/** + * Checks whether the given statement is a jump statement. + * @param {Statement} statement + * @returns {statement is JumpStatement} + */ +function isJumpStatement(statement: TSESTree.Statement) { + return ( + statement.type === 'ReturnStatement' || + statement.type === 'ContinueStatement' || + statement.type === 'BreakStatement' + ); +} + +function isSkipExpression( + node: TSESTree.Node +): node is + | TSESTree.TSInstantiationExpression + | TSESTree.TSNonNullExpression + | TSESTree.TSAsExpression + | TSESTree.TSSatisfiesExpression + | TSESTree.TSTypeAssertion + | TSESTree.ChainExpression { + return ( + node.type === 'TSInstantiationExpression' || + node.type === 'TSNonNullExpression' || + node.type === 'TSAsExpression' || + node.type === 'TSSatisfiesExpression' || + node.type === 'TSTypeAssertion' || + node.type === 'ChainExpression' + ); +} diff --git a/packages/eslint-plugin-svelte/src/type-defs/@eslint-community/eslint-utils.d.ts b/packages/eslint-plugin-svelte/src/type-defs/@eslint-community/eslint-utils.d.ts index 474538df0..3e0282fff 100644 --- a/packages/eslint-plugin-svelte/src/type-defs/@eslint-community/eslint-utils.d.ts +++ b/packages/eslint-plugin-svelte/src/type-defs/@eslint-community/eslint-utils.d.ts @@ -1,15 +1,18 @@ +import type { + CALL, + CONSTRUCT, + ESM, + READ, + StaticValue, + TraceMap, + TrackedReferences, + ReferenceTrackerOptions +} from '../../../node_modules/@eslint-community/eslint-utils/index.mjs'; +import type { AST } from 'svelte-eslint-parser'; +import type { TSESTree } from '@typescript-eslint/types'; +import type { Scope } from '@typescript-eslint/scope-manager'; + declare module '@eslint-community/eslint-utils' { - import type { AST } from 'svelte-eslint-parser'; - import type { TSESTree } from '@typescript-eslint/types'; - import type { Scope } from '@typescript-eslint/scope-manager'; - import type { - CALL, - CONSTRUCT, - ESM, - READ, - TraceMap - } from '@eslint-community/eslint-utils/referenceTracker'; - export { ReferenceTracker, TrackedReferences } from '../../../node_modules/@types/eslint-utils'; type Token = { type: string; value: string }; export function isArrowToken(token: Token): boolean; export function isCommaToken(token: Token): boolean; @@ -88,4 +91,8 @@ declare module '@eslint-community/eslint-utils' { traceMap: TraceMap ): IterableIterator>; } + export function getStaticValue( + node: TSESTree.Node, + initialScope?: Scope | null | undefined + ): StaticValue | null; } diff --git a/packages/eslint-plugin-svelte/src/utils/rules.ts b/packages/eslint-plugin-svelte/src/utils/rules.ts index 446c49b1a..bc62cc8c6 100644 --- a/packages/eslint-plugin-svelte/src/utils/rules.ts +++ b/packages/eslint-plugin-svelte/src/utils/rules.ts @@ -49,6 +49,7 @@ import noSpacesAroundEqualSignsInAttribute from '../rules/no-spaces-around-equal import noStoreAsync from '../rules/no-store-async.js'; import noSvelteInternal from '../rules/no-svelte-internal.js'; import noTargetBlank from '../rules/no-target-blank.js'; +import noTopLevelBrowserGlobals from '../rules/no-top-level-browser-globals.js'; import noTrailingSpaces from '../rules/no-trailing-spaces.js'; import noUnknownStyleDirectiveProperty from '../rules/no-unknown-style-directive-property.js'; import noUnnecessaryStateWrap from '../rules/no-unnecessary-state-wrap.js'; @@ -127,6 +128,7 @@ export const rules = [ noStoreAsync, noSvelteInternal, noTargetBlank, + noTopLevelBrowserGlobals, noTrailingSpaces, noUnknownStyleDirectiveProperty, noUnnecessaryStateWrap, diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/test01-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/test01-errors.yaml new file mode 100644 index 000000000..63e4b8cff --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/test01-errors.yaml @@ -0,0 +1,4 @@ +- message: Unexpected top-level browser global variable "window". + line: 2 + column: 12 + suggestions: null diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/test01-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/test01-input.svelte new file mode 100644 index 000000000..d6b7eee7e --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/test01-input.svelte @@ -0,0 +1,4 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/effect01-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/effect01-input.svelte new file mode 100644 index 000000000..cc4ecd464 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/effect01-input.svelte @@ -0,0 +1,6 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/env01-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/env01-input.svelte new file mode 100644 index 000000000..9ec1e8c38 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/env01-input.svelte @@ -0,0 +1,8 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/env02-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/env02-input.svelte new file mode 100644 index 000000000..8bf76db6d --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/env02-input.svelte @@ -0,0 +1,8 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/on-mount01-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/on-mount01-input.svelte new file mode 100644 index 000000000..7065d04e9 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/on-mount01-input.svelte @@ -0,0 +1,8 @@ + diff --git a/packages/eslint-plugin-svelte/tests/src/rules/no-top-level-browser-globals.ts b/packages/eslint-plugin-svelte/tests/src/rules/no-top-level-browser-globals.ts new file mode 100644 index 000000000..c13228031 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/src/rules/no-top-level-browser-globals.ts @@ -0,0 +1,16 @@ +import { RuleTester } from '../../utils/eslint-compat.js'; +import rule from '../../../src/rules/no-top-level-browser-globals.js'; +import { loadTestCases } from '../../utils/utils.js'; + +const tester = new RuleTester({ + languageOptions: { + ecmaVersion: 'latest', + sourceType: 'module' + } +}); + +tester.run( + 'no-top-level-browser-globals', + rule as any, + loadTestCases('no-top-level-browser-globals') +); From b4b1ccc0ca861811bf3b1c798a48bcdc08cbaa90 Mon Sep 17 00:00:00 2001 From: Yosuke Ota Date: Fri, 2 May 2025 15:03:04 +0900 Subject: [PATCH 02/10] Create popular-jokes-tell.md --- .changeset/popular-jokes-tell.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/popular-jokes-tell.md diff --git a/.changeset/popular-jokes-tell.md b/.changeset/popular-jokes-tell.md new file mode 100644 index 000000000..5bea6a68a --- /dev/null +++ b/.changeset/popular-jokes-tell.md @@ -0,0 +1,5 @@ +--- +"eslint-plugin-svelte": minor +--- + +feat: add `svelte/no-top-level-browser-globals` rule From 94eb4fb0434c78ad4b23fcbb9919dce4f354f00c Mon Sep 17 00:00:00 2001 From: yosuke ota Date: Fri, 2 May 2025 15:22:39 +0900 Subject: [PATCH 03/10] update --- .../src/rules/no-top-level-browser-globals.ts | 9 ++-- .../invalid/guards01-errors.yaml | 40 ++++++++++++++ .../invalid/guards01-input.svelte | 52 +++++++++++++++++++ .../invalid/test02-errors.yaml | 4 ++ .../invalid/test02-input.svelte | 3 ++ .../valid/guards01-input.svelte | 52 +++++++++++++++++++ .../eslint-plugin-svelte/tests/utils/utils.ts | 4 +- 7 files changed, 160 insertions(+), 4 deletions(-) create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards01-errors.yaml create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards01-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/test02-errors.yaml create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/test02-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards01-input.svelte diff --git a/packages/eslint-plugin-svelte/src/rules/no-top-level-browser-globals.ts b/packages/eslint-plugin-svelte/src/rules/no-top-level-browser-globals.ts index ff643f300..50c9f1e03 100644 --- a/packages/eslint-plugin-svelte/src/rules/no-top-level-browser-globals.ts +++ b/packages/eslint-plugin-svelte/src/rules/no-top-level-browser-globals.ts @@ -38,7 +38,9 @@ export default createRule('no-top-level-browser-globals', { } function enterFunction(node: TSESTree.FunctionLike) { - functions.push(node); + if (isTopLevelLocation(node)) { + functions.push(node); + } } function verifyGlobalReferences() { @@ -94,12 +96,13 @@ export default createRule('no-top-level-browser-globals', { yield ref.node; } else if (ref.node.type === 'ImportSpecifier') { const variable = findVariable(context, ref.node.local); - if (variable) + if (variable) { for (const reference of variable.references) { if (reference.isRead() && reference.identifier.type === 'Identifier') { yield reference.identifier; } } + } } } } @@ -220,7 +223,7 @@ export default createRule('no-top-level-browser-globals', { return null; } const staticValue = getStaticValue( - pp.left === node ? pp.right : pp.left, + pp.left === parent ? pp.right : pp.left, getScope(context, node) ); if (!staticValue) { diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards01-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards01-errors.yaml new file mode 100644 index 000000000..907f89791 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards01-errors.yaml @@ -0,0 +1,40 @@ +- message: Unexpected top-level browser global variable "location". + line: 5 + column: 15 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 10 + column: 15 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 15 + column: 15 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 18 + column: 15 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 23 + column: 15 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 30 + column: 15 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 35 + column: 15 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 38 + column: 15 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 43 + column: 15 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 50 + column: 15 + suggestions: null diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards01-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards01-input.svelte new file mode 100644 index 000000000..42de05fcf --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards01-input.svelte @@ -0,0 +1,52 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/test02-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/test02-errors.yaml new file mode 100644 index 000000000..247c0c4a8 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/test02-errors.yaml @@ -0,0 +1,4 @@ +- message: Unexpected top-level browser global variable "location". + line: 2 + column: 14 + suggestions: null diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/test02-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/test02-input.svelte new file mode 100644 index 000000000..90b80b1bb --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/test02-input.svelte @@ -0,0 +1,3 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards01-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards01-input.svelte new file mode 100644 index 000000000..aaf4ad2c1 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards01-input.svelte @@ -0,0 +1,52 @@ + diff --git a/packages/eslint-plugin-svelte/tests/utils/utils.ts b/packages/eslint-plugin-svelte/tests/utils/utils.ts index 845ea656f..ee2c438a8 100644 --- a/packages/eslint-plugin-svelte/tests/utils/utils.ts +++ b/packages/eslint-plugin-svelte/tests/utils/utils.ts @@ -21,7 +21,9 @@ const globals = { setInterval: 'readonly', queueMicrotask: 'readonly', window: 'readonly', - globalThis: 'readonly' + globalThis: 'readonly', + location: 'readonly', + document: 'readonly' }; /** * Prevents leading spaces in a multiline template literal from appearing in the resulting string From 05519434baa0c512542ff83101c06b0a5f1feec2 Mon Sep 17 00:00:00 2001 From: yosuke ota Date: Fri, 2 May 2025 15:38:50 +0900 Subject: [PATCH 04/10] update --- .../src/rules/no-top-level-browser-globals.ts | 67 +++---------------- .../invalid/guards02-errors.yaml | 16 +++++ .../invalid/guards02-input.svelte | 22 ++++++ .../invalid/test03-errors.yaml | 4 ++ .../invalid/test03-input.svelte | 4 ++ .../valid/guards02-input.svelte | 22 ++++++ .../eslint-plugin-svelte/tests/utils/utils.ts | 15 +---- 7 files changed, 80 insertions(+), 70 deletions(-) create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards02-errors.yaml create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards02-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/test03-errors.yaml create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/test03-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards02-input.svelte diff --git a/packages/eslint-plugin-svelte/src/rules/no-top-level-browser-globals.ts b/packages/eslint-plugin-svelte/src/rules/no-top-level-browser-globals.ts index 50c9f1e03..c20ba6023 100644 --- a/packages/eslint-plugin-svelte/src/rules/no-top-level-browser-globals.ts +++ b/packages/eslint-plugin-svelte/src/rules/no-top-level-browser-globals.ts @@ -1,5 +1,5 @@ import type { TrackedReferences } from '@eslint-community/eslint-utils'; -import { ReferenceTracker, getStaticValue, getPropertyName } from '@eslint-community/eslint-utils'; +import { ReferenceTracker, getStaticValue } from '@eslint-community/eslint-utils'; import { createRule } from '../utils/index.js'; import globals from 'globals'; import type { TSESTree } from '@typescript-eslint/types'; @@ -50,7 +50,7 @@ export default createRule('no-top-level-browser-globals', { }); type MaybeGuard = { - referenceNode: TSESTree.Node; + reference?: { node: TSESTree.Node; name: string }; isAvailableLocation: (node: TSESTree.Node) => boolean; // The guard that checks whether the browser environment is set to true. browserEnvironment: boolean; @@ -62,10 +62,10 @@ export default createRule('no-top-level-browser-globals', { * Checks whether the node is in a location where the expression is available or not. * @returns `true` if the expression is available. */ - function isAvailableLocation({ node }: { node: TSESTree.Node }) { + function isAvailableLocation(ref: { node: TSESTree.Node; name: string }) { for (const guard of maybeGuards.reverse()) { - if (guard.browserEnvironment || equalNode(guard.referenceNode, node)) { - if (guard.isAvailableLocation(node)) { + if (guard.isAvailableLocation(ref.node)) { + if (guard.browserEnvironment || guard.reference?.name === ref.name) { guard.used = true; return true; } @@ -114,7 +114,6 @@ export default createRule('no-top-level-browser-globals', { const guardChecker = getGuardChecker({ node: referenceNode }); if (guardChecker) { maybeGuards.push({ - referenceNode, isAvailableLocation: guardChecker, browserEnvironment: true }); @@ -139,7 +138,7 @@ export default createRule('no-top-level-browser-globals', { if (guardChecker) { const name = ref.path.join('.'); maybeGuards.push({ - referenceNode: ref.node, + reference: { node: ref.node, name }, isAvailableLocation: guardChecker, browserEnvironment: name === 'window' || name === 'document' }); @@ -149,15 +148,14 @@ export default createRule('no-top-level-browser-globals', { } for (const ref of reportCandidates) { - if (isAvailableLocation({ node: ref.node })) { + const name = ref.path.join('.'); + if (isAvailableLocation({ node: ref.node, name })) { continue; } context.report({ node: ref.node, messageId: 'unexpectedGlobal', - data: { - name: ref.path.join('.') - } + data: { name } }); } } @@ -300,34 +298,6 @@ export default createRule('no-top-level-browser-globals', { } return null; } - - /** - * Checks whether or not the two given nodes are same. - * @param a A node 1 to compare. - * @param b A node 2 to compare. - */ - function equalNode(a: TSESTree.Node, b: TSESTree.Node) { - if (a.type === 'Identifier' && b.type === 'Identifier') { - const leftVar = findVariable(context, a); - const rightVar = findVariable(context, b); - return leftVar && rightVar && leftVar === rightVar; - } - if (a.type === 'MemberExpression' && b.type === 'MemberExpression') { - if (!equalNode(a.object, b.object)) { - return false; - } - const leftKey = getPropertyName(a); - const rightKey = getPropertyName(b); - return leftKey && rightKey && leftKey === rightKey; - } - if (isSkipExpression(a)) { - return equalNode(a.expression, b); - } - if (isSkipExpression(b)) { - return equalNode(a, b.expression); - } - return false; - } } }); @@ -379,22 +349,3 @@ function isJumpStatement(statement: TSESTree.Statement) { statement.type === 'BreakStatement' ); } - -function isSkipExpression( - node: TSESTree.Node -): node is - | TSESTree.TSInstantiationExpression - | TSESTree.TSNonNullExpression - | TSESTree.TSAsExpression - | TSESTree.TSSatisfiesExpression - | TSESTree.TSTypeAssertion - | TSESTree.ChainExpression { - return ( - node.type === 'TSInstantiationExpression' || - node.type === 'TSNonNullExpression' || - node.type === 'TSAsExpression' || - node.type === 'TSSatisfiesExpression' || - node.type === 'TSTypeAssertion' || - node.type === 'ChainExpression' - ); -} diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards02-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards02-errors.yaml new file mode 100644 index 000000000..2d829d83c --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards02-errors.yaml @@ -0,0 +1,16 @@ +- message: Unexpected top-level browser global variable "location". + line: 5 + column: 15 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 10 + column: 15 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 15 + column: 15 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 18 + column: 15 + suggestions: null diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards02-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards02-input.svelte new file mode 100644 index 000000000..f02de6661 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards02-input.svelte @@ -0,0 +1,22 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/test03-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/test03-errors.yaml new file mode 100644 index 000000000..63d8ba42a --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/test03-errors.yaml @@ -0,0 +1,4 @@ +- message: Unexpected top-level browser global variable "localStorage". + line: 2 + column: 12 + suggestions: null diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/test03-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/test03-input.svelte new file mode 100644 index 000000000..895ae2616 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/test03-input.svelte @@ -0,0 +1,4 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards02-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards02-input.svelte new file mode 100644 index 000000000..ce58a5882 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards02-input.svelte @@ -0,0 +1,22 @@ + diff --git a/packages/eslint-plugin-svelte/tests/utils/utils.ts b/packages/eslint-plugin-svelte/tests/utils/utils.ts index ee2c438a8..cfd4d81a9 100644 --- a/packages/eslint-plugin-svelte/tests/utils/utils.ts +++ b/packages/eslint-plugin-svelte/tests/utils/utils.ts @@ -11,20 +11,11 @@ import { Linter } from 'eslint'; import * as svelteParser from 'svelte-eslint-parser'; import * as typescriptParser from '@typescript-eslint/parser'; import Module from 'module'; +import globals from 'globals'; const __dirname = path.dirname(new URL(import.meta.url).pathname); const require = Module.createRequire(import.meta.url); -const globals = { - console: 'readonly', - setTimeout: 'readonly', - setInterval: 'readonly', - queueMicrotask: 'readonly', - window: 'readonly', - globalThis: 'readonly', - location: 'readonly', - document: 'readonly' -}; /** * Prevents leading spaces in a multiline template literal from appearing in the resulting string */ @@ -250,7 +241,7 @@ function writeFixtures( [`svelte/${ruleName}`]: ['error', ...(options || [])] }, languageOptions: { - globals, + globals: globals.browser, ecmaVersion: 2020, sourceType: 'module', ...verifyConfig?.languageOptions, @@ -334,7 +325,7 @@ function getConfig(ruleName: string, inputFile: string) { { ...config, languageOptions: { - globals, + globals: globals.browser, ecmaVersion: 2020, sourceType: 'module', ...config?.languageOptions, From a30197465ca4d14dd1f2374d4565d047630d5ebb Mon Sep 17 00:00:00 2001 From: yosuke ota Date: Fri, 2 May 2025 16:37:32 +0900 Subject: [PATCH 05/10] add test and refactor --- .../src/rules/no-top-level-browser-globals.ts | 45 +++++++------- .../invalid/env01-errors.yaml | 8 +++ .../invalid/env01-input.svelte | 15 +++++ .../invalid/env02-errors.yaml | 8 +++ .../invalid/env02-input.svelte | 15 +++++ .../invalid/guards01-errors.yaml | 4 ++ .../invalid/guards01-input.svelte | 5 ++ .../invalid/guards03-errors.yaml | 60 +++++++++++++++++++ .../invalid/guards03-input.svelte | 59 ++++++++++++++++++ .../invalid/guards04-errors.yaml | 12 ++++ .../invalid/guards04-input.svelte | 17 ++++++ .../invalid/guards05-errors.yaml | 8 +++ .../invalid/guards05-input.svelte | 5 ++ .../invalid/guards06-errors.yaml | 8 +++ .../invalid/guards06-input.svelte | 18 ++++++ .../invalid/guards07-errors.yaml | 8 +++ .../invalid/guards07-input.svelte | 35 +++++++++++ .../valid/guards03-input.svelte | 52 ++++++++++++++++ .../valid/guards04-input.svelte | 17 ++++++ .../valid/guards05-input.svelte | 5 ++ .../valid/guards06-input.svelte | 18 ++++++ .../valid/guards07-input.svelte | 26 ++++++++ 22 files changed, 428 insertions(+), 20 deletions(-) create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/env01-errors.yaml create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/env01-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/env02-errors.yaml create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/env02-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards03-errors.yaml create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards03-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards04-errors.yaml create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards04-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards05-errors.yaml create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards05-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards06-errors.yaml create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards06-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards07-errors.yaml create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards07-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards03-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards04-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards05-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards06-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards07-input.svelte diff --git a/packages/eslint-plugin-svelte/src/rules/no-top-level-browser-globals.ts b/packages/eslint-plugin-svelte/src/rules/no-top-level-browser-globals.ts index c20ba6023..94c2b2d1e 100644 --- a/packages/eslint-plugin-svelte/src/rules/no-top-level-browser-globals.ts +++ b/packages/eslint-plugin-svelte/src/rules/no-top-level-browser-globals.ts @@ -81,12 +81,14 @@ export default createRule('no-top-level-browser-globals', { for (const ref of referenceTracker.iterateEsmReferences({ 'esm-env': { [ReferenceTracker.ESM]: true, + // See https://www.npmjs.com/package/esm-env BROWSER: { [ReferenceTracker.READ]: true } }, '$app/environment': { [ReferenceTracker.ESM]: true, + // See https://svelte.dev/docs/kit/$app-environment#browser browser: { [ReferenceTracker.READ]: true } @@ -107,6 +109,22 @@ export default createRule('no-top-level-browser-globals', { } } + /** + * Iterate over the used references of global variables. + */ + function* iterateBrowserGlobalReferences(): Iterable> { + yield* referenceTracker.iterateGlobalReferences( + Object.fromEntries( + blowerGlobals.map((name) => [ + name, + { + [ReferenceTracker.READ]: true + } + ]) + ) + ); + } + // Collects guarded location checkers by checking module references // that can check the browser environment. for (const referenceNode of iterateBrowserCheckerModuleReferences()) { @@ -123,16 +141,7 @@ export default createRule('no-top-level-browser-globals', { const reportCandidates: TrackedReferences[] = []; // Collects references to global variables. - for (const ref of referenceTracker.iterateGlobalReferences({ - ...Object.fromEntries( - blowerGlobals.map((name) => [ - name, - { - [ReferenceTracker.READ]: true - } - ]) - ) - })) { + for (const ref of iterateBrowserGlobalReferences()) { if (!isTopLevelLocation(ref.node)) continue; const guardChecker = getGuardCheckerFromReference(ref.node); if (guardChecker) { @@ -189,9 +198,8 @@ export default createRule('no-top-level-browser-globals', { if (!operand) return null; const staticValue = getStaticValue(operand, getScope(context, operand)); - if (!staticValue) { - return null; - } + if (!staticValue) return null; + if (staticValue.value === undefined && node.type === 'MemberExpression') { if (parent.operator === '!==' || parent.operator === '!=') { // e.g. if (globalThis.window !== undefined), if (globalThis.window != undefined) @@ -224,9 +232,8 @@ export default createRule('no-top-level-browser-globals', { pp.left === parent ? pp.right : pp.left, getScope(context, node) ); - if (!staticValue) { - return null; - } + if (!staticValue) return null; + if (staticValue.value !== 'undefined' && staticValue.value !== 'object') { return null; } @@ -264,9 +271,8 @@ export default createRule('no-top-level-browser-globals', { not?: boolean; }): ((node: TSESTree.Node) => boolean) | null { const parent = guardInfo.node.parent; - if (!parent) { - return null; - } + if (!parent) return null; + if (parent.type === 'ConditionalExpression') { const block = guardInfo.not ? parent.alternate : parent.consequent; return (n) => block.range[0] <= n.range[0] && n.range[1] <= block.range[1]; @@ -279,7 +285,6 @@ export default createRule('no-top-level-browser-globals', { const block = parent.consequent; return (n) => block.range[0] <= n.range[0] && n.range[1] <= block.range[1]; } - // e.g. if (typeof x === 'undefined') if (parent.alternate) { const block = parent.alternate; return (n) => block.range[0] <= n.range[0] && n.range[1] <= block.range[1]; diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/env01-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/env01-errors.yaml new file mode 100644 index 000000000..1c1b0d997 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/env01-errors.yaml @@ -0,0 +1,8 @@ +- message: Unexpected top-level browser global variable "location". + line: 8 + column: 15 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 13 + column: 15 + suggestions: null diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/env01-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/env01-input.svelte new file mode 100644 index 000000000..98cf30b68 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/env01-input.svelte @@ -0,0 +1,15 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/env02-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/env02-errors.yaml new file mode 100644 index 000000000..1c1b0d997 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/env02-errors.yaml @@ -0,0 +1,8 @@ +- message: Unexpected top-level browser global variable "location". + line: 8 + column: 15 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 13 + column: 15 + suggestions: null diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/env02-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/env02-input.svelte new file mode 100644 index 000000000..124d00c83 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/env02-input.svelte @@ -0,0 +1,15 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards01-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards01-errors.yaml index 907f89791..d02090e05 100644 --- a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards01-errors.yaml +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards01-errors.yaml @@ -38,3 +38,7 @@ line: 50 column: 15 suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 55 + column: 15 + suggestions: null diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards01-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards01-input.svelte index 42de05fcf..7a520431c 100644 --- a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards01-input.svelte +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards01-input.svelte @@ -49,4 +49,9 @@ } else { console.log(location.href); // NG } + if ('undefined' !== typeof window) { + console.log(location.href); + } else { + console.log(location.href); // NG + } diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards03-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards03-errors.yaml new file mode 100644 index 000000000..b6c66c354 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards03-errors.yaml @@ -0,0 +1,60 @@ +- message: Unexpected top-level browser global variable "location". + line: 5 + column: 15 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 10 + column: 15 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 15 + column: 15 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 18 + column: 15 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 25 + column: 15 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 28 + column: 15 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 35 + column: 15 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 38 + column: 15 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 42 + column: 6 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 44 + column: 15 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 46 + column: 15 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 48 + column: 6 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 50 + column: 15 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 52 + column: 15 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 57 + column: 15 + suggestions: null diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards03-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards03-input.svelte new file mode 100644 index 000000000..3d39e87c9 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards03-input.svelte @@ -0,0 +1,59 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards04-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards04-errors.yaml new file mode 100644 index 000000000..aee75b03c --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards04-errors.yaml @@ -0,0 +1,12 @@ +- message: Unexpected top-level browser global variable "location". + line: 5 + column: 15 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 10 + column: 15 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 15 + column: 15 + suggestions: null diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards04-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards04-input.svelte new file mode 100644 index 000000000..0512877d0 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards04-input.svelte @@ -0,0 +1,17 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards05-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards05-errors.yaml new file mode 100644 index 000000000..b519bfe9b --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards05-errors.yaml @@ -0,0 +1,8 @@ +- message: Unexpected top-level browser global variable "location". + line: 3 + column: 31 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 4 + column: 25 + suggestions: null diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards05-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards05-input.svelte new file mode 100644 index 000000000..f2ad9a9c8 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards05-input.svelte @@ -0,0 +1,5 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards06-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards06-errors.yaml new file mode 100644 index 000000000..6f17d46f4 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards06-errors.yaml @@ -0,0 +1,8 @@ +- message: Unexpected top-level browser global variable "location". + line: 9 + column: 15 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 13 + column: 16 + suggestions: null diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards06-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards06-input.svelte new file mode 100644 index 000000000..8177f7e20 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards06-input.svelte @@ -0,0 +1,18 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards07-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards07-errors.yaml new file mode 100644 index 000000000..542b91ae8 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards07-errors.yaml @@ -0,0 +1,8 @@ +- message: Unexpected top-level browser global variable "location". + line: 13 + column: 15 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 17 + column: 16 + suggestions: null diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards07-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards07-input.svelte new file mode 100644 index 000000000..0b36ee3a2 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards07-input.svelte @@ -0,0 +1,35 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards03-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards03-input.svelte new file mode 100644 index 000000000..86d47e6eb --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards03-input.svelte @@ -0,0 +1,52 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards04-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards04-input.svelte new file mode 100644 index 000000000..e42820f31 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards04-input.svelte @@ -0,0 +1,17 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards05-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards05-input.svelte new file mode 100644 index 000000000..6bbaabf75 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards05-input.svelte @@ -0,0 +1,5 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards06-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards06-input.svelte new file mode 100644 index 000000000..c414702c7 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards06-input.svelte @@ -0,0 +1,18 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards07-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards07-input.svelte new file mode 100644 index 000000000..add9f9744 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards07-input.svelte @@ -0,0 +1,26 @@ + From 281322a26b99eb7682e654df2ad55b941d3f83f3 Mon Sep 17 00:00:00 2001 From: yosuke ota Date: Fri, 2 May 2025 16:39:33 +0900 Subject: [PATCH 06/10] fix test --- .../invalid/guards07-errors.yaml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards07-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards07-errors.yaml index 542b91ae8..0dfb9d5c5 100644 --- a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards07-errors.yaml +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards07-errors.yaml @@ -6,3 +6,11 @@ line: 17 column: 16 suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 28 + column: 16 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 33 + column: 15 + suggestions: null From e7671e4633f45bc21201271f17bc29c7f106cefc Mon Sep 17 00:00:00 2001 From: yosuke ota Date: Fri, 2 May 2025 16:41:27 +0900 Subject: [PATCH 07/10] update doc --- docs/rules/no-top-level-browser-globals.md | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/docs/rules/no-top-level-browser-globals.md b/docs/rules/no-top-level-browser-globals.md index d7f6e4966..a82fd583f 100644 --- a/docs/rules/no-top-level-browser-globals.md +++ b/docs/rules/no-top-level-browser-globals.md @@ -26,18 +26,24 @@ This rule helps prevent the use of browser global variables that can cause error /* ✓ GOOD */ onMount(() => { - const a = window.localStorage.getItem('myCat'); + const a = localStorage.getItem('myCat'); console.log(a); }); /* ✓ GOOD */ if (browser) { - const a = window.localStorage.getItem('myCat'); + const a = localStorage.getItem('myCat'); + console.log(a); + } + + /* ✓ GOOD */ + if (typeof localStorage !== 'undefined') { + const a = localStorage.getItem('myCat'); console.log(a); } /* ✗ BAD */ - const a = window.localStorage.getItem('myCat'); + const a = localStorage.getItem('myCat'); console.log(a); ``` From d538a0f8bc96ca1948656c062adf3581a2ca1556 Mon Sep 17 00:00:00 2001 From: yosuke ota Date: Fri, 2 May 2025 17:34:24 +0900 Subject: [PATCH 08/10] add support for import.meta.env.SSR --- .../src/rules/no-top-level-browser-globals.ts | 197 ++++++++++-------- .../@eslint-community/eslint-utils.d.ts | 8 + .../invalid/env03-errors.yaml | 4 + .../invalid/env03-input.svelte | 7 + .../valid/env03-input.svelte | 7 + 5 files changed, 138 insertions(+), 85 deletions(-) create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/env03-errors.yaml create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/env03-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/env03-input.svelte diff --git a/packages/eslint-plugin-svelte/src/rules/no-top-level-browser-globals.ts b/packages/eslint-plugin-svelte/src/rules/no-top-level-browser-globals.ts index 94c2b2d1e..2ed25720d 100644 --- a/packages/eslint-plugin-svelte/src/rules/no-top-level-browser-globals.ts +++ b/packages/eslint-plugin-svelte/src/rules/no-top-level-browser-globals.ts @@ -26,16 +26,21 @@ export default createRule('no-top-level-browser-globals', { } const blowerGlobals = getBrowserGlobals(); - const functions: TSESTree.FunctionLike[] = []; + const referenceTracker = new ReferenceTracker(sourceCode.scopeManager.globalScope!, { + // Specifies the global variables that are allowed to prevent `window.window` from being iterated over. + globalObjectNames: ['globalThis'] + }); - function isTopLevelLocation(node: TSESTree.Node) { - for (const func of functions) { - if (func.range[0] <= node.range[0] && node.range[1] <= func.range[1]) { - return false; - } - } - return true; - } + type MaybeGuard = { + reference?: { node: TSESTree.Node; name: string }; + isAvailableLocation: (node: TSESTree.Node) => boolean; + // The guard that checks whether the browser environment is set to true. + browserEnvironment: boolean; + used?: boolean; + }; + const maybeGuards: MaybeGuard[] = []; + + const functions: TSESTree.FunctionLike[] = []; function enterFunction(node: TSESTree.FunctionLike) { if (isTopLevelLocation(node)) { @@ -43,88 +48,29 @@ export default createRule('no-top-level-browser-globals', { } } - function verifyGlobalReferences() { - const referenceTracker = new ReferenceTracker(sourceCode.scopeManager.globalScope!, { - // Specifies the global variables that are allowed to prevent `window.window` from being iterated over. - globalObjectNames: ['globalThis'] - }); - - type MaybeGuard = { - reference?: { node: TSESTree.Node; name: string }; - isAvailableLocation: (node: TSESTree.Node) => boolean; - // The guard that checks whether the browser environment is set to true. - browserEnvironment: boolean; - used?: boolean; - }; - const maybeGuards: MaybeGuard[] = []; - - /** - * Checks whether the node is in a location where the expression is available or not. - * @returns `true` if the expression is available. - */ - function isAvailableLocation(ref: { node: TSESTree.Node; name: string }) { - for (const guard of maybeGuards.reverse()) { - if (guard.isAvailableLocation(ref.node)) { - if (guard.browserEnvironment || guard.reference?.name === ref.name) { - guard.used = true; - return true; - } + function enterMetaProperty(node: TSESTree.MetaProperty) { + if (node.meta.name !== 'import' || node.property.name !== 'meta') return; + for (const ref of referenceTracker.iteratePropertyReferences(node, { + env: { + // See https://vite.dev/guide/ssr#conditional-logic + SSR: { + [ReferenceTracker.READ]: true } } - return false; - } - - /** - * Iterate over the references of modules that can check the browser environment. - */ - function* iterateBrowserCheckerModuleReferences(): Iterable { - for (const ref of referenceTracker.iterateEsmReferences({ - 'esm-env': { - [ReferenceTracker.ESM]: true, - // See https://www.npmjs.com/package/esm-env - BROWSER: { - [ReferenceTracker.READ]: true - } - }, - '$app/environment': { - [ReferenceTracker.ESM]: true, - // See https://svelte.dev/docs/kit/$app-environment#browser - browser: { - [ReferenceTracker.READ]: true - } - } - })) { - if (ref.node.type === 'Identifier' || ref.node.type === 'MemberExpression') { - yield ref.node; - } else if (ref.node.type === 'ImportSpecifier') { - const variable = findVariable(context, ref.node.local); - if (variable) { - for (const reference of variable.references) { - if (reference.isRead() && reference.identifier.type === 'Identifier') { - yield reference.identifier; - } - } - } + })) { + if (ref.node.type === 'Identifier' || ref.node.type === 'MemberExpression') { + const guardChecker = getGuardChecker({ node: ref.node, not: true }); + if (guardChecker) { + maybeGuards.push({ + isAvailableLocation: guardChecker, + browserEnvironment: true + }); } } } + } - /** - * Iterate over the used references of global variables. - */ - function* iterateBrowserGlobalReferences(): Iterable> { - yield* referenceTracker.iterateGlobalReferences( - Object.fromEntries( - blowerGlobals.map((name) => [ - name, - { - [ReferenceTracker.READ]: true - } - ]) - ) - ); - } - + function verifyGlobalReferences() { // Collects guarded location checkers by checking module references // that can check the browser environment. for (const referenceNode of iterateBrowserCheckerModuleReferences()) { @@ -171,9 +117,90 @@ export default createRule('no-top-level-browser-globals', { return { ':function': enterFunction, + MetaProperty: enterMetaProperty, 'Program:exit': verifyGlobalReferences }; + /** + * Checks whether the node is in a location where the expression is available or not. + * @returns `true` if the expression is available. + */ + function isAvailableLocation(ref: { node: TSESTree.Node; name: string }) { + for (const guard of maybeGuards.reverse()) { + if (guard.isAvailableLocation(ref.node)) { + if (guard.browserEnvironment || guard.reference?.name === ref.name) { + guard.used = true; + return true; + } + } + } + return false; + } + + /** + * Checks whether the node is in a top-level location. + * @returns `true` if the node is in a top-level location. + */ + function isTopLevelLocation(node: TSESTree.Node) { + for (const func of functions) { + if (func.range[0] <= node.range[0] && node.range[1] <= func.range[1]) { + return false; + } + } + return true; + } + + /** + * Iterate over the references of modules that can check the browser environment. + */ + function* iterateBrowserCheckerModuleReferences(): Iterable { + for (const ref of referenceTracker.iterateEsmReferences({ + 'esm-env': { + [ReferenceTracker.ESM]: true, + // See https://www.npmjs.com/package/esm-env + BROWSER: { + [ReferenceTracker.READ]: true + } + }, + '$app/environment': { + [ReferenceTracker.ESM]: true, + // See https://svelte.dev/docs/kit/$app-environment#browser + browser: { + [ReferenceTracker.READ]: true + } + } + })) { + if (ref.node.type === 'Identifier' || ref.node.type === 'MemberExpression') { + yield ref.node; + } else if (ref.node.type === 'ImportSpecifier') { + const variable = findVariable(context, ref.node.local); + if (variable) { + for (const reference of variable.references) { + if (reference.isRead() && reference.identifier.type === 'Identifier') { + yield reference.identifier; + } + } + } + } + } + } + + /** + * Iterate over the used references of global variables. + */ + function* iterateBrowserGlobalReferences(): Iterable> { + yield* referenceTracker.iterateGlobalReferences( + Object.fromEntries( + blowerGlobals.map((name) => [ + name, + { + [ReferenceTracker.READ]: true + } + ]) + ) + ); + } + /** * If the node is a reference used in a guard clause that checks if the node is in a browser environment, * it returns information about the expression that checks if the browser variable is available. diff --git a/packages/eslint-plugin-svelte/src/type-defs/@eslint-community/eslint-utils.d.ts b/packages/eslint-plugin-svelte/src/type-defs/@eslint-community/eslint-utils.d.ts index 3e0282fff..e4e6c3ab4 100644 --- a/packages/eslint-plugin-svelte/src/type-defs/@eslint-community/eslint-utils.d.ts +++ b/packages/eslint-plugin-svelte/src/type-defs/@eslint-community/eslint-utils.d.ts @@ -90,6 +90,14 @@ declare module '@eslint-community/eslint-utils' { public iterateGlobalReferences( traceMap: TraceMap ): IterableIterator>; + + /** + * Iterate the property references for a given expression AST node. + */ + public iteratePropertyReferences( + node: TSESTree.Expression, + traceMap: TraceMap + ): IterableIterator>; } export function getStaticValue( node: TSESTree.Node, diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/env03-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/env03-errors.yaml new file mode 100644 index 000000000..c40513248 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/env03-errors.yaml @@ -0,0 +1,4 @@ +- message: Unexpected top-level browser global variable "location". + line: 3 + column: 15 + suggestions: null diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/env03-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/env03-input.svelte new file mode 100644 index 000000000..373f21648 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/env03-input.svelte @@ -0,0 +1,7 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/env03-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/env03-input.svelte new file mode 100644 index 000000000..513c6b096 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/env03-input.svelte @@ -0,0 +1,7 @@ + From ecc1d2c7cc5aed9089a8969f589d29c60bfbc93f Mon Sep 17 00:00:00 2001 From: yosuke ota Date: Fri, 2 May 2025 18:15:28 +0900 Subject: [PATCH 09/10] refactor --- .../src/rules/no-top-level-browser-globals.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/eslint-plugin-svelte/src/rules/no-top-level-browser-globals.ts b/packages/eslint-plugin-svelte/src/rules/no-top-level-browser-globals.ts index 2ed25720d..0da78a3f5 100644 --- a/packages/eslint-plugin-svelte/src/rules/no-top-level-browser-globals.ts +++ b/packages/eslint-plugin-svelte/src/rules/no-top-level-browser-globals.ts @@ -17,13 +17,10 @@ export default createRule('no-top-level-browser-globals', { unexpectedGlobal: 'Unexpected top-level browser global variable "{{name}}".' }, type: 'problem', - conditions: [] + conditions: [{ svelteFileTypes: ['.svelte', '.svelte.[js|ts]'] }] }, create(context) { const sourceCode = context.sourceCode; - if (!sourceCode.parserServices.isSvelte && !sourceCode.parserServices.isSvelteScript) { - return {}; - } const blowerGlobals = getBrowserGlobals(); const referenceTracker = new ReferenceTracker(sourceCode.scopeManager.globalScope!, { From 61bb8af11d4bcdc035a10aeb2e584ca5679b2618 Mon Sep 17 00:00:00 2001 From: yosuke ota Date: Fri, 16 May 2025 09:12:00 +0900 Subject: [PATCH 10/10] add support for `.?` and `&&` --- .../src/rules/no-top-level-browser-globals.ts | 19 ++++++++++++++++-- .../invalid/guards08-errors.yaml | 20 +++++++++++++++++++ .../invalid/guards08-input.svelte | 8 ++++++++ .../valid/env-guards01-input.svelte | 7 +++++++ .../valid/guards08-input.svelte | 8 ++++++++ 5 files changed, 60 insertions(+), 2 deletions(-) create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards08-errors.yaml create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards08-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/env-guards01-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards08-input.svelte diff --git a/packages/eslint-plugin-svelte/src/rules/no-top-level-browser-globals.ts b/packages/eslint-plugin-svelte/src/rules/no-top-level-browser-globals.ts index 0da78a3f5..a3a94e6bc 100644 --- a/packages/eslint-plugin-svelte/src/rules/no-top-level-browser-globals.ts +++ b/packages/eslint-plugin-svelte/src/rules/no-top-level-browser-globals.ts @@ -33,7 +33,6 @@ export default createRule('no-top-level-browser-globals', { isAvailableLocation: (node: TSESTree.Node) => boolean; // The guard that checks whether the browser environment is set to true. browserEnvironment: boolean; - used?: boolean; }; const maybeGuards: MaybeGuard[] = []; @@ -126,7 +125,6 @@ export default createRule('no-top-level-browser-globals', { for (const guard of maybeGuards.reverse()) { if (guard.isAvailableLocation(ref.node)) { if (guard.browserEnvironment || guard.reference?.name === ref.name) { - guard.used = true; return true; } } @@ -280,6 +278,14 @@ export default createRule('no-top-level-browser-globals', { } if (node.type === 'MemberExpression') { + if ( + ((parent.type === 'CallExpression' && parent.callee === node) || + (parent.type === 'MemberExpression' && parent.object === node)) && + parent.optional + ) { + // e.g. globalThis.location?.href + return (n) => n === node; + } // e.g. if (globalThis.window) return getGuardChecker({ node }); } @@ -325,6 +331,15 @@ export default createRule('no-top-level-browser-globals', { return (n) => start <= n.range[0] && n.range[1] <= end; } + if ( + !guardInfo.not && + parent.type === 'LogicalExpression' && + parent.operator === '&&' && + parent.left === guardInfo.node + ) { + const block = parent.right; + return (n) => block.range[0] <= n.range[0] && n.range[1] <= block.range[1]; + } return null; } } diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards08-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards08-errors.yaml new file mode 100644 index 000000000..8e2a6880c --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards08-errors.yaml @@ -0,0 +1,20 @@ +- message: Unexpected top-level browser global variable "location". + line: 3 + column: 21 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 3 + column: 49 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 5 + column: 14 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 5 + column: 37 + suggestions: null +- message: Unexpected top-level browser global variable "location". + line: 7 + column: 14 + suggestions: null diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards08-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards08-input.svelte new file mode 100644 index 000000000..094bc97d8 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/invalid/guards08-input.svelte @@ -0,0 +1,8 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/env-guards01-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/env-guards01-input.svelte new file mode 100644 index 000000000..312b9b4fb --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/env-guards01-input.svelte @@ -0,0 +1,7 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards08-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards08-input.svelte new file mode 100644 index 000000000..5afe336b0 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-top-level-browser-globals/valid/guards08-input.svelte @@ -0,0 +1,8 @@ +