diff --git a/.claude/skills/typing-m-files/SKILL.md b/.claude/skills/typing-m-files/SKILL.md new file mode 100644 index 000000000000..cb660adec4e2 --- /dev/null +++ b/.claude/skills/typing-m-files/SKILL.md @@ -0,0 +1,127 @@ +--- +name: typing-m-files +description: Two separate jobs for `m_*.ts` files under `packages/devextreme/js/__internal`. Type: bring a file (or a line range of one) to 0 strict ESLint errors without changing behaviour. Rename: once a typed file is merged at 0, drop its `m_` prefix in a rename-only PR. Use when someone asks to type an `m_` file or part of it, fix the strict lint of an `m_` file, or rename a typed `m_` file. +--- + +# Typing `m_` files + +An `m_*.ts` file under `js/__internal` was migrated from JS, and gets relaxed lint rules (mostly `warn`). Its **strict errors** and **warnings** are what it gets once the `m_` prefix is gone. The typing work brings a file to 0 strict errors while it still has the `m_` prefix. It also fixes the warnings whose fix provably behaves the same. + +**Type or rename, never both.** The developer asks for one of the two: +- **Type** a file or a range: this file, from "Input" on. +- **Rename** a typed file: [rename.md](rename.md). + +They are always two PRs from two branches, one after the other. PRs are squash-merged into `main`, so separate commits in one PR don't help: the squashed commit holds the rename and the rewrite together, and git then shows a delete plus an add. The rename branch is made from `main` after the typing PR has been merged; for a file typed in parts, after every part's PR. If you're asked for both at once, do the typing and say the rename comes after the merge. + +**Area rules:** for a file under `js/__internal/grids/`, also follow [grids.md](grids.md). + +## Input + +The developer gives the files one per line, in the format of the typing checklists. There is a line range when only a part of a file is the task: + +``` +`packages/devextreme/js/__internal/grids/grid_core/selection/m_selection.ts`, lines 1–542 +`packages/devextreme/js/__internal/grids/tree_list/m_focus.ts` +``` + +- Without `lines`, the task is the whole file. +- A line may also name the part and its first and last member: `` `…/m_columns_view.ts`, lines 553–825 (part 2 of 4: `ColumnsView.renderDelayedTemplates` … `ColumnsView._renderCells`) ``. Check that the range starts at the first member and ends with the last one. If it doesn't, the lines have moved since the range was picked. Find the members, and tell the developer the new range before starting. +- Without member names, if a range starts or ends inside a class member or a statement, tell the developer before starting. +- Use the same format whenever you name a file or a range in your report. + +## Count strict errors and warnings + +```bash +# from the repo root; paths may be repo-relative, packages/devextreme-relative or absolute +node .claude/skills/typing-m-files/scripts/strict-lint.mjs \ + '`packages/devextreme/js/__internal/grids/grid_core/selection/m_selection.ts`, lines 1–542' \ + '`packages/devextreme/js/__internal/grids/tree_list/m_focus.ts`' \ + [--summary] [--errors] [--json] [--check] +``` + +The script takes the input lines as they are, including a trailing `(part …)` and a leading `- [ ] `. Put them in single quotes, or the shell runs the backticks. Several lines can go in one argument. The short form `path/m_x.ts:1-542` works too. + +For each file the script prints its strict errors and its warnings (inside the range, if given), a count per rule for each, and each message. `--errors` leaves the warning messages out of the list. It also shows how many errors inline disables hide; that number is for information only. `--check` exits with 1 while any errors are left; warnings don't fail it. It takes the strict rules from `packages/devextreme/eslint.config.mjs` each time it runs, so its numbers stay correct when the config changes. + +Not counted at all: `max-classes-per-file`, `simple-import-sort/imports` and `spellcheck/spell-checker` (kept on purpose). + +## Workflow + +1. **List the work.** Run the script on the file or range. Its errors and warnings are what the task fixes. +2. **Type, in this order.** + 1. Class fields. Only in a whole file, or in the part that holds the class fields (usually the first): every later method depends on them. + 2. Signatures: param types and return types. + 3. The remaining rules inside bodies. + 4. Warnings, one at a time, only where the fix provably behaves the same (see Warnings below). + + [fixes.md](fixes.md) has the team's fix for each rule, most frequent first, and a section on warnings. +3. **Review your own diff against [traps.md](traps.md).** Every body edit must keep behaviour exactly the same. +4. **Checks** (below). +5. **Report:** + - strict errors and warnings, before and after, for each file or range in the input format; + - every body edit, one line each, with why it behaves the same; + - what was added to the module's `types.ts`; + - the warnings left, grouped by rule, with the reason each was left. + +## Rules + +**Scope** +- The task's files are the given files plus the module's own `types.ts`, where new types go (create it if the module has none). With a range, touch only those lines of the given file, plus its import block. +- Anything else needs the developer's OK before you touch it: the test file for an agreed refactor, another module's `types.ts`, or callers in other files. +- If a new type (usually a return type) breaks callers in other files, stop and ask the developer. The options are a compile-only fix at those callers, a wider type, or agreeing it with whoever is typing those files. +- Don't change public API: no edits to `js/**/*.d.ts`. + +**Typing only** +- Annotate params, locals, returns and fields. +- Change a body only where a new type makes it fail to compile (a guard, a `?? fallback`, a narrowing), or where a strict rule needs it (`fixes.md`). Either way, the edit must provably behave the same for every value the code can get. A value from a source that isn't typed yet gets a directive, not a cast (see Disables). +- Don't refactor. That means no extracting functions, rewriting loops, renaming members or functions, removing dead code or dropping unused params, unless the developer agrees for that one function. Renaming a local variable to fix `no-shadow` is fine. +- An agreed refactor gets tests first: written and passing on the old code, then the change, then a mutation check that the tests catch a broken version. The test file is outside the task: name it when you ask for the agreement. +- A bug found while typing is not fixed in the typing PR. Tell the developer, with file, line and why it's a bug. + +**Warnings** +- The target is 0 errors. Warnings are best effort. +- Fix a warning when the fix provably behaves the same, for every value the code can get. "Provably" means you can show it from the types or the code: a trustworthy type (not `any`), or the surrounding code. A test run is not proof. +- Otherwise leave the warning as it is, and don't add a disable for it. +- `fixes.md` ("Warnings") says when each warning rule's fix behaves the same. + +**Where types go** +- Reuse before creating: public types from the `js/**/*.d.ts` files (`@js/common`, `@js/core/utils/callbacks`, the component's own `@js/ui/`, …), then the module's own `types.ts`. +- A new type goes into the module's own `types.ts`. Other modules import it from there. + +**Disables** +- Existing inline `eslint-disable` comments and `@ts-expect-error` may stay. +- **A value from a source that isn't typed yet gets a directive, not a cast.** A cast stays silent once the source gets typed. A directive is then reported as unused, so someone sees it and removes it. + - The value is `any`, which is what `no-unsafe-return` reports: use `// eslint-disable-next-line @typescript-eslint/no-unsafe-return -- `. Don't use `@ts-expect-error` here: TypeScript sees no error on an `any` value, so tsc fails at once with TS2578 (unused directive). + - TypeScript rejects the value, because it is `unknown` or has a wider or wrong type: use `// @ts-expect-error `. Keep that line short, because the directive hides every error on it. +- Remove a directive once it is reported as unused: TS2578 from tsc, or "Unused eslint-disable directive" from ESLint. ESLint shows that as a warning, and `strict-lint.mjs` shows it as a note. +- Any other new disable is a last resort: line-level, with a reason (`-- `). Never a file-level disable. + +**Style** +- Braces around every `if` body, including one-line guards. +- No `_` prefix on new members. +- No new comments, except a short one that says *why*. + +**Never** +- Run `pnpm run dev`. The developer runs it and tells you when the build is ready. +- Edit the generated wrappers (`packages/devextreme-{angular,react,vue}/src`), `localization/default_messages.ts` or `cldr-data`. +- Commit or push without being asked. + +## Checks before a PR + +Run from `packages/devextreme`: + +```bash +node ../../.claude/skills/typing-m-files/scripts/strict-lint.mjs --check '' # 0 errors; the warnings left go in the report +pnpm exec eslint # no errors +pnpm exec tsc --noEmit -p js/__internal/tsconfig.json; echo "exit $?" # exit 0 +``` + +- `tsc`: use `js/__internal/tsconfig.json`. The root `tsconfig.json` compiles only `.d.ts` files, so it passes no matter what. In a fresh worktree, run `pnpm nx build:localization:generate devextreme` first, or you get phantom TS2307 errors for `cldr-data`. +- With a range, only the range must be at 0. The rest of the file keeps its errors until its own part. +- Tests: only those that cover the changed code. Pick them by tracing what uses it, not by folder. Whole-tree runs happen on CI. + - Jest (Jest 30, the flag is plural): + ```bash + pnpm exec jest --no-coverage --runInBand --selectProjects jsdom-tests --testPathPatterns "" + ``` + - QUnit (`testing/tests//…`) runs on the developer's dev server. Ask them to start it and tell you when the build is ready. Then open `http://localhost:20060/run/?notimers=true&nojquery=true&nocsp=true`. + - Before trusting a QUnit result, check the built file of *every* file you changed, under `packages/devextreme/artifacts/transpiled/`. The watch build sometimes skips one without saying so. The built file must be newer than the source and must contain your body edits. Type-only edits leave no trace in the built JS. If a built file is stale, ask the developer to rebuild (`rm -rf .nx/cache`, then restart the dev build). diff --git a/.claude/skills/typing-m-files/fixes.md b/.claude/skills/typing-m-files/fixes.md new file mode 100644 index 000000000000..68f29a23fd67 --- /dev/null +++ b/.claude/skills/typing-m-files/fixes.md @@ -0,0 +1,160 @@ +# Fixes per rule + +These are the fixes used in earlier typing PRs, most frequent rule first. The examples come from the grids typing PRs, but the idioms apply to any `m_` file; grid-only idioms are in [grids.md](grids.md). The shares were measured on the 10,856 strict errors of the grids' `m_` files. The last section covers warnings, which are fixed only where the fix provably behaves the same. + +Compiler settings to keep in mind (`js/__internal/tsconfig.json`): +- `strict` is on, so `strictNullChecks` is on too. +- `noImplicitAny` is off, so an untyped param is silently `any`. ESLint is the only thing that catches it. +- A new return type is a new contract for every caller. Before you narrow one, check its callers across `js/__internal`. + +## Missing return type: 40% + +`explicit-function-return-type` and `explicit-module-boundary-types` (return part). One missing return type is often reported by both rules, so one fix removes two errors. + +What TypeScript infers at these places today, measured on a sample: + +| Inferred | Share | What to write | +|---|--:|---| +| `void` | 41% | `: void` | +| `any` | 30% | a real type: find what the body really returns | +| a primitive (`boolean`, `number`, `string`) | 12% | that type | +| a named type | 10% | that type | +| an object literal or a long type | 5% | a named type in the module's `types.ts` | + +```ts +- public isColumnOptionUsed(optionName): any { ++ public isColumnOptionUsed(optionName: string): boolean { +``` +- `callbackNames()`/`publicMethods()` return `string[]`. +- A value that is only passed along returns `unknown`. +- Type guards: `): sortOrder is SortOrder =>`, `.filter((key): key is string | number => …)`. +- An inline callback passed to a typed API (`forEach`, `map`, `.done(...)`) needs no annotation. +- A standalone arrow does: `const hasAdditionalFilter = (): boolean => …`. + +## Missing param type: 14% + +`explicit-module-boundary-types` ("Argument … should be typed"). Take the type from the module's `types.ts` or the public d.ts. Type the params in place. Turning a `function` expression into an arrow isn't needed for typing. It behaves the same only if the function uses neither `this` nor `arguments` and is never called with `new`. The team's earlier PRs did it like this: +```ts +-export const getChildrenByBandColumn = function (columnIndex, columnChildrenByIndex, recursive) { ++export const getChildrenByBandColumn = ( ++ columnIndex: number | undefined, ++ columnChildrenByIndex: BandColumnsCache['columnChildrenByIndex'], ++ recursive: boolean, ++): Column[] => { +``` +- `| undefined` on a param describes the existing contract when the body already guards `undefined` and callers pass possibly-undefined values. It is not a widening. +- `sortOrder` params use `SortOrder` from `@js/common`, plus `| 'none'` where the method accepts it. + +## `no-unsafe-return` 11%, `no-explicit-any` 9% + +Replace `any` with: +- a specific union (`string | null | undefined`, `number | undefined`); +- `unknown`; +- a handler type (`(e: unknown) => void`); +- a public type (`DataType`, `HorizontalAlignment`, `Format`, `DeferredObj`, `Callback<[…]>`, the component's own types from `@js/ui/`). + +A value from a source that isn't typed yet gets a directive, not a cast. A cast stays silent once the source gets typed. A directive is then reported as unused, and gets removed. +```ts +- return this._dataSource[optionName]() as number; ++ // eslint-disable-next-line @typescript-eslint/no-unsafe-return -- ++ return this._dataSource[optionName](); +``` +- For an `any` value, use `eslint-disable`, as above. `@ts-expect-error` doesn't work there: TypeScript sees no error on an `any` value, so tsc fails at once with TS2578 (unused directive). +- Use `// @ts-expect-error ` only when TypeScript itself rejects the value, because it is `unknown` or has a wider or wrong type. +- The team's earlier PRs used casts here (`… as number`, `… as DeferredObj`). Don't copy them. + +`unknown` instead of `any`: +- Use `unknown` when callers only store the value or pass it on. `unknown` makes the code check the value before using a member of it. +- When callers use its members, `unknown` breaks every call site (`'x' is of type 'unknown'`), and those are often in files outside the task. Give it its real type instead. For example, the earlier `dataSource(): any` is now `DataSourceController.getDataSource(): DataSource | null`. +- Keep `any` with `// eslint-disable-next-line @typescript-eslint/no-explicit-any -- ` only when neither works. + +## `@stylistic/max-len`: 5% + +The limit is 100. Comments count, strings and template literals don't. Put one param per line, and start a wrapped condition with `&&`/`||`. If a disable comment itself is too long, disable max-len for it: +```ts +// eslint-disable-next-line @stylistic/max-len +// eslint-disable-next-line @typescript-eslint/no-unsafe-return, @typescript-eslint/explicit-module-boundary-types +``` + +## `init-declarations`: 3% + +Move the first assignment into the declaration, when nothing reads the variable before it: +```ts +- let rowspan; ++ let rowspan: number = that.getRowCount(); +``` +- Otherwise, for example when an if-chain or a closure assigns it, keep a line disable with a reason: `// eslint-disable-next-line @typescript-eslint/init-declarations -- assigned in `. +- Don't add a placeholder value. `= 0` changes what an early read gets, and `= undefined` is forbidden by `no-undef-init`. +- Moving an if-chain into a helper so the variable can be `const` (`const changeType = resolveChangeType(optionName);`) is a refactor: only when agreed. + +## `no-this-alias`: 3% + +Use `this` instead of `that`, but only inside arrow callbacks. Inside a `function` callback, `this` is something else. +```ts +- when(that._columnsController.applyDataSource(dataSource)).done(() => { ++ when(this._columnsController.applyDataSource(dataSource)).done(() => { +``` +`function (isLoading) { … }.bind(this)` becomes `(isLoading: boolean): void => { … }`, if the body doesn't use `arguments`. + +## `prefer-rest-params` / `prefer-spread`: 2% + +Call `super` with named params, when the method declares exactly those params and neither it nor the base reads `arguments`. With named params, a call with extra arguments drops them: +```ts +- protected _processItems(items, change) { +- items = super._processItems.apply(this, arguments as any); ++ protected _processItems(items: RawItemData[], change: DataChange): ProcessedItem[] { ++ const processedItems = super._processItems(items, change); +``` +- Otherwise forward everything with a rest param: `super.x(...args)`. +- A variadic method gets overloads plus a rest param: `private filter(...filterArgs: [] | [DataFilter] | BinaryDataFilterExpression)`. +- Forwarding: `result.resolve.apply(result, arguments)` becomes `.done((...args: unknown[]) => { result.resolve(...args); })`. +- `arguments.length` is not flagged. If the code depends on it (a getter/setter overload), keep the `function` and its `arguments.length` check. +- `const callBase = super.x.bind(this);` is fine. + +## Smaller rules + +| Rule | Fix | +|---|---| +| `no-plusplus` | `i += 1`. `` `dx-col-${id++}` `` becomes `` `dx-col-${id}` `` followed by `id += 1;` | +| `no-param-reassign` | a default param (`rowIndex = 0` instead of `rowIndex = rowIndex \|\| 0`, only when the old code treats only `undefined` as missing; see traps), or a new `const`. Mutating an object's properties is allowed | +| `no-unused-expressions` | `cond && doIt()` becomes `if (cond) { doIt(); }`, which always behaves the same. `a.update && a.update(x)` becomes `a.update?.(x)` only when `a.update` can only be a function, `null` or `undefined`. If it can be `false`, `0` or `''`, the old code skips the call but `?.()` throws, so use `if (a.update) { a.update(x); }` | +| `@stylistic/no-mixed-operators` | parentheses that spell out the current grouping: `a && b \|\| c` becomes `(a && b) \|\| c`, since `&&` binds tighter. A named boolean also works: `const isDefaultCommandColumn = column.command && !isCustom(column); if (isDefaultCommandColumn \|\| !column.fixedPosition)` | +| `no-shadow` | rename the inner name: `(column) =>` becomes `(commandColumn) =>` | +| `max-depth` (3) | at function level: early `return` guards, or merging nested `if`s with `&&`, where the flow stays the same. Inside a loop there is no such fix, because `no-continue` is a strict rule too. Then ask the developer: an agreed refactor (a helper, a loop rewrite), or a line disable with a reason | +| `consistent-return` | an explicit `return undefined;` at the end, and `default: return undefined;` in a switch: the function already returned `undefined` there. Not `return false;`, which changes the value | +| `prefer-for-of` | `for (const column of this._columns)`, when the index is only used to read the item and the loop doesn't add or remove items | +| `no-use-before-define` | move the declaration above its first use, when that doesn't change evaluation order | +| `no-multi-assign` | two statements; `x = o.x = o.x === undefined ? d : o.x` becomes `o.x ??= d;` then `x = o.x`, only if `o.x` can't be `null` | +| `no-non-null-assertion` | a narrowing TypeScript can see, when the value provably isn't `null` or `undefined` there. If it can be, the old code threw or read a key named `'undefined'`, so a check with a new fallback (`oldItem ? map[getRowKey(oldItem)] : undefined`) changes behaviour. Ask, or keep a line disable with a reason | +| `no-invalid-this` | a typed `this` param, which changes nothing: `function (this: ColumnBase, e: ColumnCustomizeTextArg): string`. An arrow changes what `this` is, so use one only after replacing every `this` with what it was. Core `each(items, function () {…})` calls the callback with `this` set to the item, and stops when it returns `false`, so keep `each` rather than switching to `forEach` | +| `no-floating-promises` | a line disable with a reason. `void promise` is not an option: `no-void` forbids it | + +## Class fields + +Fields typed as `any` (part 1 of a file, or a whole file): +- Assigned in `init()`: definite assignment, `public _columns!: Column[];`. +- Lazy or cached: optional, `private _visibleColumns?: Column[][];`. Write `?: boolean`, not `: boolean | undefined`. +- Callbacks: a typed tuple, `public pageChanged!: Callback<[number?]>;`. +- A narrower type for an inherited field in a subclass: `public declare _dataSource?: GroupingDataSourceAdapter | null;`. +- Drop `readonly` when utils assign the field. A field that is overridden becomes `protected`, not `private`. +- Once a field is typed, remove the casts of it (`(this._columns as Column[])`). + +## `@ts-expect-error` + +- A new one needs a reason: `// @ts-expect-error ownerBand holds the band column until updateColumnIndexes sets its index`. +- Prefer it to a cast when TypeScript rejects a value from a source that isn't typed yet. For an `any` value, use `eslint-disable` instead (see `no-unsafe-return` above). +- Remove one when tsc reports it as unused (TS2578). That means its source has been typed. + +## Warnings (best effort) + +The target is 0 errors. Fix a warning only when you can show that the fix behaves the same for every value the code can get. Otherwise leave it, with no disable. Counts are for the grids' `m_` files after the rename, to show which rules matter most. + +| Rule | Count | The fix behaves the same when | +|---|--:|---| +| `prefer-nullish-coalescing` | 618 | the left side's type can't be `0`, `''`, `false` or `NaN`, and that type is trustworthy (not `any`, not from a cast). `x \|\| d` → `x ?? d` and `x = x \|\| d` → `x ??= d` differ exactly for those values (see traps.md) | +| `prefer-optional-chain` | 360 | the expression is only used as a condition (`if`, `!`, `&&`/`\|\|` inside a condition), or `a` can only be an object, a function, `null` or `undefined`. In a value position, `a && a.b` gives `a` when `a` is `0`, `''` or `false`, but `a?.b` gives `undefined`. This rule is off for `m_` files, so it only appears after the rename | +| `func-names` | 221 | always, if the new name isn't used in the function's body (inside the function, the name would hide an outer variable of the same name) and nothing reads the function's `.name` | +| `no-restricted-globals` | 39 | all of them are `setTimeout`. The rule asks to keep a `setTimeout` only when there is no other way, with a comment saying why. If you know why, write `// eslint-disable-next-line no-restricted-globals -- `. Replacing the timer changes behaviour, so not in a typing PR. If you don't know why, leave it | +| `require-await` | 4 | only in a Jest test callback. Removing `async` turns a rejected promise into a plain throw, and both fail the test. Elsewhere, leave it | + +In test files, `explicit-function-return-type`, `no-explicit-any` and `no-unsafe-return` are warnings after the rename, not errors. Fix them as described above. diff --git a/.claude/skills/typing-m-files/grids.md b/.claude/skills/typing-m-files/grids.md new file mode 100644 index 000000000000..40403ffb159f --- /dev/null +++ b/.claude/skills/typing-m-files/grids.md @@ -0,0 +1,79 @@ +# Grids: extra rules for `js/__internal/grids/` + +These apply on top of `SKILL.md` to files under `js/__internal/grids/`: `grid_core`, `data_grid`, `tree_list`, `pivot_grid` and CardView (`new/`). + +## Dependency direction + +- `data_grid`, `tree_list`, `pivot_grid` and CardView are built on `grid_core`. **`grid_core` never imports from them.** +- Inside `grid_core`, a module never adds an import from a module that depends on it. +- Reach another module's controllers and views the way the code already does: through `getController('x')`/`getView('x')`. The `Controllers`/`Views` registry in `grid_core/m_types.ts` types them, and no new import is needed. + +## Where types go + +- Reuse before creating: + - `Controllers['x']`/`Views['x']`, `ModuleType`, `OptionChanged`/`OptionChangedFor` from `grid_core/m_types.ts`; + - public types from `@js/common/grids` and `@js/ui/data_grid` (`ColumnBase`, `FixedPosition`, `Properties as DataGridProperties`, …). +- **Leaks.** Sometimes feature logic has leaked into core: core code reads a row field that editing or grouping sets, or handles a view only `data_grid` registers. Declare the type core needs in the core module's `types.ts`, not by importing it from the feature, and mark it: + ```ts + /** @architectureLeak : */ + ``` + `grep -rn "@architectureLeak" js/__internal/grids` lists them all. Moving one to its owner is a separate architecture task. +- `Controllers['x']` or a direct `import type` of the class: both work, and the direct import is the more common style. `grid_core/m_types.ts` itself uses inline `import('…')` types so that it has no import cycles. Don't add a regular import to it. + +## Style enforced here + +In `grid_core`, `data_grid` and `tree_list`, ESLint requires explicit `public`/`protected`/`private` on members (constructors excepted) and forbids `get`/`set` accessors. Keep to both in `pivot_grid` too. + +## Typing idioms + +**Extenders.** Write the return type as `ModuleType`: +```ts +const summaryDataController = (Base: ModuleType): ModuleType => class SummaryDataControllerExtender extends Base { +``` +A generic mixin `>(Base: T) => class extends Base` can't name its return type. It keeps `// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types, @typescript-eslint/explicit-function-return-type`. + +**Controller fields.** `public _columnsController!: Controllers['columns'];` + +**Options** +- `this.option('a.b')` is already typed from the public options through `InternalGridOptions`, so delete the casts: `this.option('summary.groupItems') ?? []`. +- An option the module handles but the public d.ts doesn't have: declare `Options` in the module's `types.ts`, and type `optionChanged` with it: + ```ts + export type ColumnsControllerOptionChanged = OptionChanged + | OptionChangedFor + | ColumnOptionChanged; + ``` + Declare only the options the `optionChanged` switch handles. An unhandled one falls through to `super.optionChanged(args: OptionChanged)` and doesn't compile. +- Known limit: `this.option('')` still returns `never`, because `option()` knows only `InternalGridOptions`. Leave it; it is separate work. +- TS can't narrow by comparing two fields: `args.name === args.fullName` becomes `args.fullName === 'columns'`. + +## Traps + +- **Same method names on different chains.** `_changeRowExpandCore`, `_processGroupItems` and others exist on both the `DataController` chain and the `DataSourceAdapter` chain. Before typing or changing an override, check which class the enclosing extender extends. Also, an option-object property named like a method (`pageCount()` inside an options object) is a delegate, not an override. +- **Extender order is inheritance order.** `registerModulesOrder([...])` in `data_grid/m_widget_base.ts` and `tree_list/m_widget_base.ts` decides which extender wraps which. A later module is more derived. A type that assumes one extender runs before another has to follow that order. +- **Widely used returns.** The columns controller's public getters (`getColumns`, `getVisibleColumns`, `getFixedColumns`, …) are called from 51 files outside `columns_controller/`. Check the callers before narrowing one of them. +- **Grid column objects inherit fields**, so a `for…in` over a column also walks inherited keys (see traps.md). + +## Tests (only when a refactor was agreed) + +- A test for DataGrid- or TreeList-only behaviour goes in that component's `__tests__`, not in `grid_core`. +- Use the existing `createDataGrid`/`createTreeList` helpers (`__tests__/__mock__/helpers/utils.ts`); both default `keyExpr: 'id'`. For the grid's overloaded `option`, use `spyOnOption` from the same file. +- No new QUnit tests for grids: cover new code with Jest. The existing QUnit tests are only for checking behaviour. + +## Renames in grids + +On top of [rename.md](rename.md): +- **Hub files** are imported from more than 20 places outside their module. Rename them only at a moment the team has agreed and announced, so everyone merges or rebases first. Ask the developer before renaming one of these: + - `grid_core/m_types.ts`, `grid_core/m_utils.ts`, `grid_core/m_modules.ts` + - `grid_core/views/m_rows_view.ts`, `grid_core/column_headers/m_column_headers.ts` + - `pivot_grid/m_widget_utils.ts` +- `grid_core/m_types.ts` imports most controllers and views with inline `import('./…/m_x')` types. Update that line too. +- Diagram scripts: + - `grids/__docs__/scripts/data_grid/constants.ts` maps `m_*` file names to feature areas. Rename the entry. + - For `m_modules.ts` only, also `grids/__docs__/scripts/grid_core/constants.ts`: `M_MODULES_PATH` and `EXCLUDED_FILE_NAMES`. +- **`module_not_extended/` folders** have no `m_` prefix, so there is nothing to rename. They get the relaxed rules only through the `'js/__internal/**/module*/**.ts'` glob of the "Rules for migrated from JS files" block in `packages/devextreme/eslint.config.mjs`. That glob also matches `ui/html_editor/modules/` and `ui/list/modules/`, so don't delete it. Once a folder has 0 strict errors, add it to that block's `ignores`: + ```js + // Rules for migrated from JS files + { + files: ['js/__internal/**/m_*.ts', 'js/__internal/**/module*/**.ts'], + ignores: ['js/__internal/grids/**/module_not_extended/**'], + ``` diff --git a/.claude/skills/typing-m-files/rename.md b/.claude/skills/typing-m-files/rename.md new file mode 100644 index 000000000000..21d967a2708b --- /dev/null +++ b/.claude/skills/typing-m-files/rename.md @@ -0,0 +1,46 @@ +# Rename-only PR + +Once a file has 0 strict errors, it loses its `m_` prefix in a PR of its own. That PR renames and updates imports, and does nothing else. For grids files, also follow "Renames in grids" in [grids.md](grids.md). + +## When + +- The typing PR is merged into `main`. For a file typed in parts, every part's PR is merged. +- The rename is its own branch, made from the latest `main`. Don't build it on the typing branch: after a squash merge, the typing branch's commits are not in `main`, so a branch built on it shows the typing changes again and conflicts. If the current branch isn't a fresh one from the latest `main`, or it holds other changes, stop and ask the developer. Don't create or switch branches yourself. +- `strict-lint.mjs --check ''` (no range) reports 0 on that branch, before the rename. +- A file imported from many places (more than about 20 outside its module) is renamed only at a moment the team has agreed and announced, so everyone merges or rebases first. Ask the developer before renaming one. + +## Steps + +1. **Only the `m_` prefix goes.** The file stays in the same folder, and the rest of its name stays the same: `grid_core/views/m_rows_view.ts` becomes `grid_core/views/rows_view.ts`, and `m_x.test.ts` becomes `x.test.ts`. Don't move the file or change anything else in its name. + - **Check that the new name is free.** If `/x.ts` already exists, stop and ask the developer. Today that is `core/utils/m_date.ts` and `core/utils/m_math.ts`: `date.ts` and `math.ts` exist next to them. + - **Check for a folder with the new name.** If `/x/` exists, an import of `'./x'` resolves to the new file, not to the folder's `index.ts`. Check that nothing imports the folder as `'./x'` or `'../x'`, and tell the developer. Today that is `grid_core/m_utils.ts` next to `grid_core/utils/`; `m_utils.ts` imports that folder as `'./utils/index'`, which keeps working. +2. `git mv /m_x.ts /x.ts`. Don't change the file's content, so git keeps tracking it as a rename. A rename plus a rewrite shows up as a delete plus an add, and every other open PR that touches the file then conflicts. +3. Update every importer: in each import path only the last part changes, `…/m_x` to `…/x`. Search for the path without its extension, in every form it is imported by: + ```bash + cd packages/devextreme + git grep -n -E "['\"/]m_x['\"]" -- js testing + ``` + Keep only the hits that resolve to this file; other modules have files with the same name. The forms in use are: + - relative: `'./m_x'`, `'../m_x'`; + - the `@ts/` alias: `'@ts//…/m_x'`; + - QUnit tests: `'__internal//…/m_x'`; + - public entries: `js/ui/*.js` import `'../__internal//…/m_widget'`; + - `jest.mock('…/m_x', …)`; + - inline type imports: `import('./…/m_x')`. +4. A matching `m_x.test.ts` is a separate `m_` file with its own strict count. Rename it only when it is at 0 too. + +## Checks + +```bash +cd packages/devextreme +git diff -M --stat ...HEAD # e.g. upstream/main; the file must show as a rename (=>), not as a delete plus an add +pnpm exec eslint # the file now gets the strict rules: 0 errors +pnpm exec tsc --noEmit -p js/__internal/tsconfig.json; echo "exit $?" +``` + +- If `js/ui/**` re-exports the file (for example a `m_widget.ts`), also run `pnpm run update-ts-reexports` and check that nothing changed. CI fails on stale generated re-exports. +- Tests: the Jest files that import the renamed file (the `git grep` above finds them). A broken import fails the whole suite file at load time. + +## On a conflict + +Don't resolve a rename conflict by hand. Make the branch again from the latest `main`, and run the `git mv` and the import updates again. diff --git a/.claude/skills/typing-m-files/scripts/strict-lint.mjs b/.claude/skills/typing-m-files/scripts/strict-lint.mjs new file mode 100644 index 000000000000..ce2b14f943f7 --- /dev/null +++ b/.claude/skills/typing-m-files/scripts/strict-lint.mjs @@ -0,0 +1,173 @@ +#!/usr/bin/env node +// Counts the strict ESLint errors and warnings of `m_` files: what a file gets once the `m_` prefix is gone. +// +// node .claude/skills/typing-m-files/scripts/strict-lint.mjs ... [--summary] [--errors] [--json] [--check] +// +// A is given in either form (one argument may hold several lines, one file per line): +// path/m_x.ts or path/m_x.ts:1-542 +// '`path/m_x.ts`' or '`path/m_x.ts`, lines 1–542 (part 1 of 4: …)' (a checklist line; a leading `- [ ] ` is fine) +// Quote the second form in single quotes, or the shell runs the backticks. +// Paths may be absolute, relative to the current directory, to the repo root or to packages/devextreme. +// A range limits the count to those lines (a part of a split file). +// --summary counts only, no message list +// --errors list error messages only (warnings are still counted) +// --json machine-readable output +// --check exit code 1 when any file (or range) still has strict errors (warnings don't fail it) +// +// The strict rules are not hard-coded: for every file, the script asks ESLint for the config of its +// strict twin (same path, basename without `m_`, `module*` folders renamed) and applies the rules that +// differ. So the count follows `packages/devextreme/eslint.config.mjs` as it changes. +import { createRequire } from 'node:module'; +import { existsSync, readFileSync } from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const REPO = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../../../..'); +const PKG = path.join(REPO, 'packages/devextreme'); +const { ESLint } = createRequire(path.join(PKG, 'package.json'))('eslint'); + +// Kept on purpose by the typing plan, not part of the typing work. +const KEPT = new Set(['spellcheck/spell-checker', 'max-classes-per-file', 'simple-import-sort/imports']); + +const args = process.argv.slice(2); +const flags = new Set(args.filter((a) => a.startsWith('--'))); +const specs = args.filter((a) => !a.startsWith('--')); +if (specs.length === 0) { + console.error('usage: strict-lint.mjs [:from-to] | \'``, lines –\' ... [--summary] [--errors] [--json] [--check]'); + process.exit(2); +} + +const resolveFile = (p) => { + const candidates = path.isAbsolute(p) ? [p] : [path.resolve(p), path.join(REPO, p), path.join(PKG, p)]; + return candidates.find((c) => existsSync(c)); +}; + +// `path`, `path:1-542`, or a checklist line: `- [ ] \`path\`, lines 1–542 (part 1 of 4: …)` +const parseSpec = (spec) => { + const text = spec.trim().replace(/^- \[[ xX]\]\s*/, ''); + const quoted = /^`([^`]+)`(?:,\s*lines\s+(\d+)\s*[–-]\s*(\d+))?/.exec(text); + const plain = /^(.*?)(?::(\d+)\s*[–-]\s*(\d+))?$/.exec(text); + const [, file, from, to] = quoted ?? plain; + return { file, from, to }; +}; + +const targets = specs.flatMap((s) => s.split('\n')).filter((s) => s.trim()).map((spec) => { + const { file, from, to } = parseSpec(spec); + const abs = resolveFile(file); + if (!abs) { + console.error(`not found: ${file} (from "${spec.trim()}")`); + process.exit(2); + } + const rel = path.relative(PKG, abs).split(path.sep).join('/'); + const ranged = from !== undefined; + return { spec, abs, rel, from: ranged ? Number(from) : 1, to: ranged ? Number(to) : Infinity, ranged }; +}); + +const twinOf = (rel) => { + const parts = rel.split('/'); + const base = parts.pop().replace(/^m_/, ''); + const dirs = parts.map((d) => (/^module/.test(d) ? `twin_${d}` : d)); + return [...dirs, base].join('/'); +}; + +const probe = new ESLint({ cwd: PKG }); +const overrideConfig = []; +for (const t of targets) { + const own = (await probe.calculateConfigForFile(t.rel)).rules; + const strict = (await probe.calculateConfigForFile(twinOf(t.rel))).rules; + const rules = {}; + for (const [rule, value] of Object.entries(strict)) { + if (JSON.stringify(value) !== JSON.stringify(own[rule])) { + rules[rule] = value; + } + } + if (Object.keys(rules).length > 0) { + overrideConfig.push({ files: [t.rel], rules }); + } +} + +const eslint = new ESLint({ cwd: PKG, overrideConfig }); +const results = await eslint.lintFiles([...new Set(targets.map((t) => t.rel))]); +const byFile = new Map(results.map((r) => [path.relative(PKG, r.filePath).split(path.sep).join('/'), r])); + +const isStrict = (m) => m.severity === 2 && m.ruleId && !KEPT.has(m.ruleId); +const isWarning = (m) => m.severity === 1 && m.ruleId && !KEPT.has(m.ruleId); +const shortRule = (id) => id.replace('@typescript-eslint/', 'ts/').replace('@stylistic/', ''); + +const report = targets.map((t) => { + const r = byFile.get(t.rel); + const inRange = (m) => m.line >= t.from && m.line <= t.to; + const errors = r.messages.filter(isStrict); + const ranged = errors.filter(inRange); + const fatal = r.messages.filter((m) => m.fatal || (!m.ruleId && m.severity === 2)); + const warnings = r.messages.filter(isWarning); + const rangedWarnings = warnings.filter(inRange); + const other = r.messages.filter((m) => !m.ruleId && m.severity === 1); + const countByRule = (list) => { + const byRule = {}; + for (const m of list) { + byRule[m.ruleId] = (byRule[m.ruleId] ?? 0) + 1; + } + return Object.fromEntries(Object.entries(byRule).sort((a, b) => b[1] - a[1])); + }; + const toMessage = (m) => ({ line: m.line, column: m.column, rule: m.ruleId, message: m.message }); + return { + file: `packages/devextreme/${t.rel}`, + from: t.from, + to: t.ranged ? t.to : readFileSync(t.abs, 'utf8').split('\n').length, + ranged: t.ranged, + errors: ranged.length, + errorsInFile: errors.length, + warnings: rangedWarnings.length, + warningsInFile: warnings.length, + suppressed: (r.suppressedMessages ?? []).filter(isStrict).filter(inRange).length, + byRule: countByRule(ranged), + warningsByRule: countByRule(rangedWarnings), + messages: ranged.map(toMessage), + warningMessages: rangedWarnings.map(toMessage), + fatal: fatal.map((m) => `${m.line}:${m.column} ${m.message}`), + notes: other.map((m) => m.message), + }; +}); + +if (flags.has('--json')) { + console.log(JSON.stringify(report, null, 1)); +} else { + for (const f of report) { + console.log(f.ranged ? `\`${f.file}\`, lines ${f.from}–${f.to}` : `\`${f.file}\``); + const inFile = (n) => (f.ranged ? ` (${n} in the whole file)` : ''); + console.log(` strict errors: ${f.errors}${inFile(f.errorsInFile)}; hidden by inline disables: ${f.suppressed}`); + console.log(` warnings: ${f.warnings}${inFile(f.warningsInFile)}`); + for (const x of f.fatal) { + console.log(` FATAL ${x}`); + } + for (const x of f.notes) { + console.log(` note: ${x}`); + } + const rules = (byRule) => Object.entries(byRule).map(([k, v]) => `${shortRule(k)} ${v}`).join(', '); + if (f.errors > 0) { + console.log(` errors by rule: ${rules(f.byRule)}`); + } + if (f.warnings > 0) { + console.log(` warnings by rule: ${rules(f.warningsByRule)}`); + } + if (!flags.has('--summary')) { + for (const m of f.messages) { + console.log(` ${m.line}:${m.column} error ${shortRule(m.rule)} ${m.message}`); + } + if (!flags.has('--errors')) { + for (const m of f.warningMessages) { + console.log(` ${m.line}:${m.column} warn ${shortRule(m.rule)} ${m.message}`); + } + } + } + } + if (report.length > 1) { + const sum = (key) => report.reduce((a, f) => a + f[key], 0); + console.log(`total: ${sum('errors')} strict errors, ${sum('warnings')} warnings`); + } +} + +if (flags.has('--check') && report.some((f) => f.errors > 0 || f.fatal.length > 0)) { + process.exit(1); +} diff --git a/.claude/skills/typing-m-files/traps.md b/.claude/skills/typing-m-files/traps.md new file mode 100644 index 000000000000..6f3cf4547a97 --- /dev/null +++ b/.claude/skills/typing-m-files/traps.md @@ -0,0 +1,29 @@ +# Traps: edits that look safe but change behaviour + +Typing PRs must not change behaviour. Check every body edit in your diff against this list, and for grids also against the traps in [grids.md](grids.md). Green tests don't prove an edit is safe: several of these went through review and CI unnoticed. + +## Values + +- **`||` and `??` are not the same.** `x || d` replaces `0`, `''`, `false` and `NaN`; `x ?? d` doesn't. Swap them only when the value can't be one of those. The same goes for `rowIndex = rowIndex || 0` becoming a default param: a default param replaces only `undefined`. +- **`extend()` and spread are not the same.** Core `extend` skips `undefined` values; spread copies them as own keys. Swapping them adds `undefined` keys, and QUnit `deepEqual` fails even though a JSON dump looks identical. Also, spread inside a `reduce` makes it O(n²). +- **`a && a()` becomes `a?.()`** only when `a` is a function or nullish. If `a` can be `false`, `0` or `''`, the old code skipped the call and the new code throws. +- **`!!x` and `Boolean(x)` are fine; `x !== undefined` is not the same.** A guard added to make a type compile must let exactly the same values through as the old code. +- **`for…in` walks inherited keys; `Object.keys` doesn't.** Some objects inherit fields; grid column objects do, for one. Keep `for…in` (with its disable and reason) where inherited fields matter. +- **`arguments.length`** tells a getter from a setter call (`columnOption(id, name)` versus `columnOption(id, name, value)`). A rest param or a default param must keep that difference. + +## Methods and overrides + +- **Never collapse a two-hop read.** `store()?.key()` is not `key()`, and `dataSource()?.x()` is not `x()`, once some component overrides the outer method. TreeList overrides `key()`, for one. Check every override before merging two calls into one. +- **Deleting an override whose `super` call has `@ts-expect-error` breaks subclasses.** That override is also the typed link for every subclass's `super` call. The runtime still works, but tsc fails. Move the directive down to the subclass instead of deleting it. +- **Narrowing a getter in an override and casting is a smell.** Make the base generic over the thing it returns, with a default type param so other users are untouched, and put the one honest cast where the object is created. + +## Types + +- **Only a type alias is assignable to `Record`** (`RawItemData`). An interface is not, because it has no implicit index signature. That is why `GroupItem` is an alias. Don't "clean it up" into an interface. +- **A return type reaches every caller.** A widely used method can have callers in dozens of files. Returning `T | undefined` breaks callers that read a field without a check. Check the callers before narrowing, and stop and ask if they are outside the task's files. + +## Tests (only when a refactor was agreed) + +- A test that calls `instance.dispose()` must also remove its container. Otherwise every later test in the file fails in `afterTest`. +- A unit stub that answers option reads must leave out `_optionCache`. Otherwise the first value read is frozen and an option-change test fails as if the code had a bug. +- Jest 30 types the arguments of `toHaveBeenCalledWith`. For an overloaded target like a component's `option`, they collapse to `never`. Use a single-signature mock type (grids have a `spyOnOption` helper). diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 34458f09c6b3..6ab80c12d8fe 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -77,7 +77,7 @@ /packages/devextreme/js/ui/sortable.d.ts @DevExpress/devextreme-grids /packages/devextreme/js/ui/sortable.js @DevExpress/devextreme-grids -/packages/devextreme/js/__internal/m_sortable.ts @DevExpress/devextreme-grids +/packages/devextreme/js/__internal/sortable.ts @DevExpress/devextreme-grids /packages/devextreme-scss/scss/widgets/base/_sortable.scss @DevExpress/devextreme-grids /packages/devextreme-scss/scss/widgets/fluent/sortable/** @DevExpress/devextreme-grids @@ -92,7 +92,7 @@ /packages/devextreme/js/ui/draggable.d.ts @DevExpress/devextreme-grids /packages/devextreme/js/ui/draggable.js @DevExpress/devextreme-grids -/packages/devextreme/js/__internal/m_draggable.ts @DevExpress/devextreme-grids +/packages/devextreme/js/__internal/draggable.ts @DevExpress/devextreme-grids /packages/devextreme-scss/scss/widgets/base/_draggable.scss @DevExpress/devextreme-grids @@ -145,6 +145,8 @@ /packages/devextreme/testing/tests/DevExpress.ui.widgets.pivotGrid/** @DevExpress/devextreme-scheduler /packages/testcafe-models/pivotGrid/** @DevExpress/devextreme-scheduler /e2e/testcafe-devextreme/tests/pivotGrid/** @DevExpress/devextreme-scheduler +/e2e/playwright/models/pivotGrid/** @DevExpress/devextreme-scheduler +/e2e/playwright/tests/common/pivotGrid/** @DevExpress/devextreme-scheduler /packages/devextreme-scss/scss/widgets/base/pivotGrid/** @DevExpress/devextreme-scheduler /packages/devextreme-scss/scss/widgets/fluent/pivotGrid/** @DevExpress/devextreme-scheduler diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index abfb6134970c..da5d8519490c 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -168,8 +168,7 @@ pnpm nx run-many -t lint # all packages pnpm run lint # devextreme package: js, ts, dts, texts pnpm run lint-js -- --fix # auto-fix JS -# Regenerate (after changes to generators, TS declarations, or devextreme-internal-tools) -pnpm run regenerate-all +# Regenerate TS reexports and bundle (after changes to public .d.ts) pnpm run update-ts-reexports # from packages/devextreme pnpm run update-ts-bundle # from packages/devextreme @@ -183,7 +182,7 @@ pnpm nx clean:artifacts devextreme # build artifacts only ``` packages/ devextreme/ # core library: ui/, viz/, core/, data/, __internal/ - devextreme-{angular,react,vue}/ # framework wrappers (generated) + devextreme-{angular,react,vue}/ # framework wrappers (hand/AI-maintained) devextreme-scss/ # SCSS themes devextreme-themebuilder/ # theme builder devextreme-metadata/ # metadata for wrapper generation @@ -209,14 +208,14 @@ clean (`devextreme-nx-infra-plugin:clean` preserving CSS and npm metadata) → l - Use `pnpm nx ` rather than raw npm scripts so Nx caching and the dependency graph stay correct. - Build before testing: `pnpm nx build:dev devextreme`; QUnit and TestCafe both require an up-to-date build. -- Run `pnpm run regenerate-all` after editing wrapper generators, TypeScript declarations, or `devextreme-internal-tools`. -- Edit source files only under `packages/devextreme/js/**`, `packages/devextreme-scss/scss/**`, and `packages/devextreme-metadata/**`. +- The Angular/React/Vue wrappers are maintained by hand (with AI agents), not regenerated from metadata. When public API in `packages/devextreme/js/{ui,viz}/**/*.d.ts` changes, update all three wrappers in the same PR following `.github/instructions/wrapper-{angular,react,vue}.instructions.md` and `.github/instructions/public-api-wrappers.instructions.md`. +- Edit source files under `packages/devextreme/js/**`, `packages/devextreme-scss/scss/**`, `packages/devextreme-metadata/**`, and the wrapper `packages/devextreme-{angular,react,vue}/src/**` (except each package's `src/core/**`; the one exception is adding a `PROPERTY_TOKEN_` to `devextreme-angular/src/core/tokens/index.ts` for a new collection option). - Match the Node and pnpm versions declared in `package.json` (`engines`, `packageManager`); mismatched versions cause CI failure. - Set `DEVEXTREME_TEST_CI=true` for test-mode builds and `BUILD_TEST_INTERNAL_PACKAGE=true` for wrapper test prep. ## Constraints -- NEVER edit generated wrappers under `packages/devextreme-{angular,react,vue}/src/` (templates excepted); update the generators and run `pnpm run regenerate-all` instead. +- The wrappers under `packages/devextreme-{angular,react,vue}/src/` are maintained by hand — keep them in sync with the public API per the wrapper instructions; NEVER edit each package's `src/core/**` (hand-written base classes) to accommodate a single component. - NEVER hand-edit `packages/devextreme/js/__internal/core/localization/{default_messages.ts,cldr-data/**}`; regenerate via the localization executor. - NEVER run `pnpm install` without `--frozen-lockfile`; use `pnpm install --frozen-lockfile` to match CI. @@ -224,7 +223,7 @@ clean (`devextreme-nx-infra-plugin:clean` preserving CSS and npm metadata) → l - Always install with `pnpm install --frozen-lockfile`; never plain `pnpm install`. - Build before test: `pnpm nx build:dev devextreme`. -- Generated wrappers under `packages/devextreme-{angular,react,vue}/src/` are read-only — modify generators and run `pnpm run regenerate-all`. +- Wrappers under `packages/devextreme-{angular,react,vue}/src/` are hand/AI-maintained — update them per the wrapper instructions when public API changes; leave each `src/core/**` alone (except Angular's `src/core/tokens/index.ts`, see the Angular wrapper instructions). - Prefer `pnpm nx ` over direct npm scripts for caching. - Consult @.github/instructions/ for file-specific coding rules before editing. diff --git a/.github/instructions/public-api-wrappers.instructions.md b/.github/instructions/public-api-wrappers.instructions.md new file mode 100644 index 000000000000..a787e30990e0 --- /dev/null +++ b/.github/instructions/public-api-wrappers.instructions.md @@ -0,0 +1,88 @@ +--- +applyTo: "**/packages/devextreme/js/{ui,viz}/**/*.{d.ts,js}" +--- + +# Public API changes must be propagated to the framework wrappers + +The Angular, React, and Vue wrappers are **no longer regenerated** from +`devextreme-metadata` / `devextreme-internal-tools`. They are maintained by hand (with the +help of local AI agents). This means **every public API change in a wrapped component's +`.d.ts` or public `.js` JSDoc must be applied, in the same pull request, to all three +wrappers**. + +This file applies to the public API surface: the `.d.ts` files in `js/ui/**` and +`js/viz/**`, and the public `.js` entry files next to them. Those `.js` files contain only +re-exports and JSDoc blocks such as `@name dxTabsOptions.activeStateEnabled` + `@hidden`, +which also decide the wrapper surface. Use this file both when editing the API and when +reviewing an API change. + +## What counts as a wrapper-affecting change + +A change to any of the following in a component's options interface (e.g. `dxButtonOptions`, +its `Properties` alias, or a nested option type such as `dxDataGridColumn`) affects the +wrappers: + +- **Options** — a property added, removed, renamed, or retyped on the options interface. +- **Events** — an `onX` callback added, removed, renamed, or whose event type changed. +- **Event types** — a `*Event` type (e.g. `ClickEvent`) whose shape changed. +- **Nested option types** — a collection/object option type (e.g. `columns`, `items`, + `toolbar`) added, removed, renamed, or restructured. +- **Template options** — a `template` option added or removed. +- **Visibility** — an option or nested field hidden or un-hidden: `@hidden` added or removed + (including `@hidden false`) in a `.d.ts`, **or** in a `@name .