Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 1 addition & 13 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,7 @@ const {
NO_COMPILATION_REQUIRED_TS_SELECTORS,
CJS_GLOBALS_IN_ESM,
} = require('./eslint/erasable-syntax-selectors.cjs');

const DATA_TEST_SELECTORS = [
{
selector: 'Literal[value=/\\[data-test-/]',
message:
'`data-test-*` attributes are stripped in production builds. Use a plain `data-*` attribute (e.g. `[data-foo]`) for functional selectors.',
},
{
selector: 'TemplateElement[value.raw=/\\[data-test-/]',
message:
'`data-test-*` attributes are stripped in production builds. Use a plain `data-*` attribute (e.g. `[data-foo]`) for functional selectors.',
},
];
const { DATA_TEST_SELECTORS } = require('./eslint/data-test-selectors.cjs');

module.exports = {
root: true,
Expand Down
28 changes: 28 additions & 0 deletions eslint/data-test-selectors.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
'use strict';

// `no-restricted-syntax` selectors that flag `data-test-*` CSS/DOM selectors
// in app code. `data-test-*` is a test-only hook, not a functional selector
// API: host app builds strip these attributes in production (ember-test-
// selectors), and in realm-card code (compiled by runtime-common, which does
// NOT strip them) coupling styling or behavior to a test hook is fragile —
// deleting a test selector would silently change production. Either way, use a
// plain `data-*` attribute for things you actually select on.
//
// Shared by the root `.eslintrc.js` and by packages whose own config is
// `root: true` (e.g. `packages/host`, `packages/boxel-ui/addon`), which do not
// inherit the root config and so must re-declare these selectors themselves.
const DATA_TEST_MESSAGE =
"Don't select on `data-test-*`: it's a test-only attribute (host builds strip it in production; card code keeps it but coupling to a test hook is fragile). Use a plain `data-*` attribute (e.g. `[data-foo]`) for functional selectors.";

const DATA_TEST_SELECTORS = [
{
selector: 'Literal[value=/\\[data-test-/]',
message: DATA_TEST_MESSAGE,
},
{
selector: 'TemplateElement[value.raw=/\\[data-test-/]',
message: DATA_TEST_MESSAGE,
},
];

module.exports = { DATA_TEST_SELECTORS };
11 changes: 5 additions & 6 deletions packages/base/default-templates/card-info.gts
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ class CardInfoEditor extends GlimmerComponent<EditSignature> {
(getFieldIcon @model item.key)
}}
data-test-edit-preview={{item.key}}
data-edit-preview-field={{item.key}}
>
{{#if item.value}}
<Field @format='atom' />
Expand Down Expand Up @@ -200,7 +201,8 @@ class CardInfoEditor extends GlimmerComponent<EditSignature> {
{{on 'click' this.toggleThumbnailEditor}}
data-test-toggle-thumbnail-editor
>
Change Thumbnail
Change
{{#unless @hideThemeChooser}}Theme & {{/unless}}Thumbnail

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems unrelated to the lint rule, is it an inclusion from elsewhere?

Image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes sorry it’s unrelated. Noticed this regression while fixing the lint error on that file

</Button>
</div>
</:label>
Expand Down Expand Up @@ -240,10 +242,7 @@ class CardInfoEditor extends GlimmerComponent<EditSignature> {
@icon={{ImageIcon}}
data-test-field='cardInfo-thumbnailURL'
>
<div
class='thumbnail-picker'
data-thumbnail-picker-controls
>
<div class='thumbnail-picker' data-thumbnail-picker-controls>
<div class='thumbnail-picker-inputs'>
<span
class='thumbnail-picker-input-slot'
Expand Down Expand Up @@ -406,7 +405,7 @@ class CardInfoEditor extends GlimmerComponent<EditSignature> {
.null-preview {
color: var(--muted-foreground, var(--boxel-450));
}
.default-preview :deep([data-test-edit-preview='cardThumbnailURL']) {
.default-preview :deep([data-edit-preview-field='cardThumbnailURL']) {
overflow-wrap: anywhere;
min-width: 0;
}
Expand Down
15 changes: 15 additions & 0 deletions packages/boxel-ui/addon/.eslintrc.cjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
'use strict';

const MISSING_INVOKABLES_CONFIG = require('../../runtime-common/etc/eslint/missing-invokables-config');
const {
DATA_TEST_SELECTORS,
} = require('../../../eslint/data-test-selectors.cjs');

module.exports = {
root: true,
Expand Down Expand Up @@ -45,6 +48,18 @@ module.exports = {
],
},
overrides: [
{
// Disallow `data-test-*` CSS/DOM selectors in source code. ember-test-selectors
// strips these attributes in production, so selectors like
// `querySelector('[data-test-foo]')` silently break outside of tests.
// This package is `root: true`, so it cannot inherit the monorepo-root
// config and must re-declare the guard. Scoped to source only — tests
// legitimately select on `data-test-*`.
files: ['src/**/*.{js,ts,gts,gjs}'],
rules: {
'no-restricted-syntax': ['error', ...DATA_TEST_SELECTORS],
},
},
{
files: ['**/*.gts'],
parser: 'ember-eslint-parser',
Expand Down
15 changes: 15 additions & 0 deletions packages/boxel-ui/test-app/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
'use strict';

const MISSING_INVOKABLES_CONFIG = require('../../runtime-common/etc/eslint/missing-invokables-config');
const {
DATA_TEST_SELECTORS,
} = require('../../../eslint/data-test-selectors.cjs');

module.exports = {
root: true,
Expand Down Expand Up @@ -43,6 +46,18 @@ module.exports = {
],
},
overrides: [
{
// Disallow `data-test-*` CSS/DOM selectors in app code. ember-test-selectors
// strips these attributes in production, so selectors like
// `querySelector('[data-test-foo]')` silently break outside of tests.
// This package is `root: true`, so it cannot inherit the monorepo-root
// config and must re-declare the guard. Scoped to app code only — tests
// legitimately select on `data-test-*`.
files: ['app/**/*.{js,ts,gts,gjs}'],
rules: {
'no-restricted-syntax': ['error', ...DATA_TEST_SELECTORS],
},
},
{
files: ['**/*.gts'],
parser: 'ember-eslint-parser',
Expand Down
32 changes: 29 additions & 3 deletions packages/catalog/.eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,30 @@
const {
NO_COMPILATION_REQUIRED_TS_SELECTORS,
} = require('../../eslint/erasable-syntax-selectors.cjs');
const { DATA_TEST_SELECTORS } = require('../../eslint/data-test-selectors.cjs');

// contents/ files (both .ts card/command modules and .gts Glimmer components)
// always go through the realm/Ember compilation pipeline, so decorators like
// @field, @tracked, and @action are valid here — only the `Decorator` selector
// is lifted. The remaining erasable-syntax guards (enum, `import =`,
// `export =`, runtime namespaces) still apply, for consistency with the rest
// of the repo.
const ERASABLE_MINUS_DECORATOR = NO_COMPILATION_REQUIRED_TS_SELECTORS.filter(
(s) => s.selector !== 'Decorator',
);

// `data-test-*` is a test-only hook. Unlike host app builds, the realm card
// pipeline (runtime-common) does NOT strip these attributes, so card selectors
// on them survive to production — but coupling styling/behavior to a test hook
// is fragile (deleting a test selector silently changes production), so it is
// banned in card content too. Tests legitimately select on them, so the
// data-test ban is dropped for test files (see CONTENTS_TEST_RESTRICTED_SYNTAX).
const CONTENTS_RESTRICTED_SYNTAX = [
'error',
...NO_COMPILATION_REQUIRED_TS_SELECTORS.filter(
(s) => s.selector !== 'Decorator',
),
...ERASABLE_MINUS_DECORATOR,
...DATA_TEST_SELECTORS,
];
const CONTENTS_TEST_RESTRICTED_SYNTAX = ['error', ...ERASABLE_MINUS_DECORATOR];

module.exports = {
overrides: [
Expand Down Expand Up @@ -66,5 +77,20 @@ module.exports = {
'no-undef': 'off',
},
},
{
// Tests legitimately select on `data-test-*` (e.g.
// `assert.dom('[data-test-foo]')`); keep the erasable-syntax guards but
// drop the data-test ban. This override only resets `no-restricted-syntax`
// — the parser/extends from the `contents/**/*.gts` override above still
// apply to `.gts` tests.
files: [
'contents/**/*.test.{js,ts,gts,gjs}',
'contents/**/*-test.{js,ts,gts,gjs}',
'contents/tests/**',
],
rules: {
'no-restricted-syntax': CONTENTS_TEST_RESTRICTED_SYNTAX,
},
},
],
};
7 changes: 1 addition & 6 deletions packages/experiments-realm/product.gts
Original file line number Diff line number Diff line change
Expand Up @@ -320,9 +320,6 @@ class Isolated extends Component<typeof Product> {
padding: 7px 24px;
border: 0;
}
div[data-test-compound-field-format='atom'] {
display: inline-block;
}
</style>
</template>
}
Expand All @@ -348,9 +345,7 @@ export class Product extends CardDef {
});

static embedded = class Embedded extends Component<typeof this> {
<template>
<EmbeddedProductComponent @model={{@model}} />
</template>
<template><EmbeddedProductComponent @model={{@model}} /></template>
};

static isolated = Isolated;
Expand Down
13 changes: 13 additions & 0 deletions packages/host/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict';

const MISSING_INVOKABLES_CONFIG = require('../runtime-common/etc/eslint/missing-invokables-config');
const { DATA_TEST_SELECTORS } = require('../../eslint/data-test-selectors.cjs');

// Applies to all of JS, TS, GJS, and GTS in the browser context.
const sharedBrowserConfig = {
Expand Down Expand Up @@ -59,6 +60,18 @@ module.exports = {
browser: true,
},
overrides: [
{
// Disallow `data-test-*` CSS/DOM selectors in app code. ember-test-selectors
// strips these attributes in production, so selectors like
// `querySelector('[data-test-foo]')` silently break outside of tests.
// This package is `root: true`, so it cannot inherit the monorepo-root
// config and must re-declare the guard. Scoped to app code only — tests
// legitimately select on `data-test-*`.
files: ['app/**/*.{js,ts,gts,gjs}'],
rules: {
'no-restricted-syntax': ['error', ...DATA_TEST_SELECTORS],
},
},
{
files: ['**/*.{js,ts}'],
parser: '@typescript-eslint/parser',
Expand Down
6 changes: 6 additions & 0 deletions packages/software-factory/.template-lintrc.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
'use strict';

module.exports = {
Comment thread
burieberry marked this conversation as resolved.
extends: ['recommended', '@cardstack/template-lint:recommended'],
plugins: ['../template-lint/plugin'],
};
3 changes: 3 additions & 0 deletions packages/software-factory/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
"smoke:tools": "NODE_NO_WARNINGS=1 node scripts/smoke-tests/factory-tools-smoke.ts",
"lint": "concurrently \"pnpm:lint:*(!fix)\" --names \"lint:\"",
"lint:fix": "concurrently \"pnpm:lint:*:fix\" --names \"fix:\"",
"lint:hbs": "ember-template-lint realm --no-error-on-unmatched-pattern",
"lint:hbs:fix": "ember-template-lint realm --fix --no-error-on-unmatched-pattern",
"lint:js": "eslint . --report-unused-disable-directives --cache",
"lint:js:fix": "eslint . --report-unused-disable-directives --fix",
"lint:format": "prettier --check .",
Expand Down Expand Up @@ -62,6 +64,7 @@
"decorator-transforms": "catalog:",
"ember-concurrency": "catalog:",
"ember-modifier": "^4.1.0",
"ember-template-lint": "catalog:",
"eslint-plugin-qunit": "catalog:",
"opencode-ai": "1.14.34",
"fs-extra": "catalog:",
Expand Down
2 changes: 1 addition & 1 deletion packages/software-factory/realm/document.gts
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,7 @@ export class Document extends CardDef {
</button>
</div>

{{! template-lint-disable no-invalid-interactive*/}}
{{! template-lint-disable no-invalid-interactive}}
<nav class='toc-navigation' {{on 'click' this.handleTocClick}}>
{{#if @model.content}}
<div class='toc-section'>
Expand Down
4 changes: 2 additions & 2 deletions packages/software-factory/realm/test-results.gts
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,9 @@ export class TestModuleResult extends FieldDef {
<span class='module-counts'>
{{@model.passedCount}}/{{@model.totalCount}}
passed
{{#if this.args.model.skippedCount}}
{{#if @model.skippedCount}}
<span class='skipped-label'>
({{this.args.model.skippedCount}}
({{@model.skippedCount}}
skipped)
</span>
{{/if}}
Expand Down
37 changes: 30 additions & 7 deletions packages/template-lint/lib/no-data-test-selector.mjs
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
import { Rule } from 'ember-template-lint';

// data-test-* attributes are stripped from production builds by ember-test-selectors.
// Using them as CSS selectors (e.g. exceptSelector='[data-test-foo]', querySelector('[data-test-foo]'))
// works in tests but silently breaks in production.
// data-test-* is a test-only hook, not a functional selector API. Host app
// builds strip these attributes in production (ember-test-selectors), so a
// selector like exceptSelector='[data-test-foo]' / find-all('[data-test-foo]')
// silently breaks there. Realm cards (compiled by runtime-common) do NOT strip
// them, but coupling styling/behavior to a test hook is still fragile —
// deleting a test selector would silently change production. Either way, use a
// plain data-* attribute for things you actually select on.
const MESSAGE =
"Don't select on `data-test-*`: it's a test-only attribute (host builds strip it in production; card code keeps it but coupling to a test hook is fragile). Use a plain `data-*` attribute (e.g. `[data-foo]`) for functional selectors.";

export default class NoDataTestSelector extends Rule {
visitor() {
return {
Expand All @@ -12,8 +19,7 @@ export default class NoDataTestSelector extends Rule {
StringLiteral(node) {
if (/\[data-test-/.test(node.value)) {
this.log({
message:
'`data-test-*` attributes are stripped in production builds. Use a plain `data-*` attribute (e.g. `[data-foo]`) for functional selectors.',
message: MESSAGE,
node,
});
}
Expand All @@ -27,12 +33,29 @@ export default class NoDataTestSelector extends Rule {
/\[data-test-/.test(node.value.chars)
) {
this.log({
message:
'`data-test-*` attributes are stripped in production builds. Use a plain `data-*` attribute (e.g. `[data-foo]`) for functional selectors.',
message: MESSAGE,
node: node.value,
});
}
},

// Catches CSS selectors in `<style>` blocks:
// <style scoped>[data-test-foo] { color: red; }</style>
// The style element's CSS is a plain TextNode child, not parsed as CSS,
// so scan its raw text for `[data-test-`.
ElementNode(node) {
if (node.tag !== 'style') {
return;
}
for (let child of node.children) {
if (child.type === 'TextNode' && /\[data-test-/.test(child.chars)) {
this.log({
message: MESSAGE,
node: child,
});
}
}
},
};
}
}
17 changes: 17 additions & 0 deletions packages/template-lint/plugin.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,23 @@ export default {
// https://github.com/ember-template-lint/ember-template-lint/blob/e1d3fd25fc1b8b250edd9bce11500f78721759c0/lib/rules/no-forbidden-elements.js#L9
'no-forbidden-elements': ['meta', 'html', 'script'],
},
overrides: [
{
// Tests legitimately select on `data-test-*` (e.g. `find-all
// '[data-test-foo]'`) — that's what the attribute is for — so the
// guard against using it as a functional selector only applies to
// non-test templates. Covers both `tests/` dirs and co-located
// `*-test.*` / `*.test.*` files.
files: [
'**/tests/**',
'**/*-test.{gjs,gts,hbs,js,ts}',
'**/*.test.{gjs,gts,hbs,js,ts}',
],
rules: {
'no-data-test-selector': false,
},
},
],
},
},
};
Loading
Loading