Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions src/eslint/quality.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,33 @@ export default [
"unicorn/prefer-ternary": "off",
"unicorn/no-useless-undefined": "off",

// --- Unicorn: rule that makes the canonical browser framing check
// unspellable. `window.parent === window` is how DOM code asks "am I
// in an iframe?", and every rewrite this rule accepts is rejected by
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
// another shipped rule:
//
// window.parent === window → unicorn/prefer-global-this
// parent === self → unicorn/prefer-global-this
// globalThis.parent === globalThis→ unicorn/no-unnecessary-global-this
// parent === globalThis → sonarjs/different-types-comparison
//
// The last one is not a style opinion: lib.dom types `parent`, `top`,
// `opener` and `MessageEvent.source` as `Window`, while `globalThis`
// is `typeof globalThis`. Those types do not overlap, so the
// type-aware rule correctly reports the comparison as always-false —
// which means there is no spelling of the check that satisfies the
// shipped set. `window` is also the *typed* global in DOM code, not a
// legacy alias.
//
// It also contradicts a rule we already ship as an error:
// `sonarjs/no-global-this` forbids the very `globalThis` that
// `prefer-global-this` demands. Turning it off makes that pair
// consistent.
//
// `unicorn/no-unnecessary-global-this` still trims `globalThis.parent`
// to `parent` for plain member access, so the useful half is kept.
"unicorn/prefer-global-this": "off",

// --- Unicorn: modernization rule that outruns our runtime. The
// suggested API isn't shipped on our Node target yet, so its
// autofix rewrites working code into a call that compiles but
Expand Down
64 changes: 64 additions & 0 deletions test/browser-framing-check.test.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import { test } from "node:test";
import assert from "node:assert/strict";
import { Linter } from "eslint";
import globals from "globals";
import unicorn from "eslint-plugin-unicorn";

import quality from "../src/eslint/quality.js";

// Guards that the canonical browser framing check stays spellable.
//
// `window.parent === window` is how DOM code asks "am I in an iframe?" before
// posting to a host. Under the upstream unicorn recommended set every rewrite
// is rejected by some other shipped rule (see the comment on
// `unicorn/prefer-global-this` in src/eslint/quality.js), so the check has no
// valid spelling and consumers reach for a suppression or an `as unknown as
// Window` bridge. We ship `prefer-global-this` off to keep it writable.

const linter = new Linter();

const configWithRuleOn = {
languageOptions: { globals: globals.browser },
plugins: { unicorn },
rules: {
"unicorn/prefer-global-this": "error",
"unicorn/no-unnecessary-global-this": "error",
},
};

const FRAMING_CHECK = `const isTopLevel = window.parent === window;\n`;

test("upstream unicorn rejects the window-based framing check", () => {
const messages = linter.verify(FRAMING_CHECK, configWithRuleOn, "frame.js");
const ruleIds = messages.map((message) => message.ruleId);

assert.ok(
ruleIds.includes("unicorn/prefer-global-this"),
`expected prefer-global-this to fire upstream, got: ${ruleIds.join(", ") || "none"}`,
);
});

test("shipped quality concern turns prefer-global-this off", () => {
assert.equal(
resolve("unicorn/prefer-global-this"),
"off",
"unicorn/prefer-global-this must stay off — see the comment in quality.js",
);
});

/** Last-wins lookup of a rule across the concern's flat-config entries. */
const resolve = (ruleId) =>
quality.findLast((entry) => entry.rules?.[ruleId] !== undefined)?.rules[
ruleId
];

test("the useful half of the pair is kept", () => {
// `globalThis.parent` must still trim to `parent` for plain member access.
assert.equal(resolve("unicorn/no-unnecessary-global-this"), "error");

// Documents the contradiction this change resolves: `prefer-global-this`
// demands `globalThis`, while `sonarjs/no-global-this` forbids it. With
// `prefer-global-this` off, the pair is consistent again.
assert.equal(resolve("sonarjs/no-global-this"), "error");
assert.equal(resolve("unicorn/prefer-global-this"), "off");
});