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
114 changes: 114 additions & 0 deletions packages/devextreme/build/pure-getters-guard.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
/**
* Guard against minifiers deleting signal dependency subscriptions (T1334012)
*
* `@preact/signals-core` subscribes as a *side effect* of reading `.value`, so a read whose result
* is never used can still be load-bearing. Terser's `compress.pure_getters` asserts that no getter
* has side effects and deletes such reads, silently unsubscribing the reaction.
*
* This is not an exotic opt-in: Angular CLI's legacy webpack builder enables
* `pure_getters` by default via `buildOptimizer`
*
* Every module is minified twice and the two outputs are compared. Any `.value` read
* that survives without `pure_getters` but disappears with it is a lost subscription.
*/
import { transformSync } from '@babel/core';
import * as fs from 'fs';
import * as path from 'path';

const terser = require(require.resolve('terser', {
paths: [path.dirname(require.resolve('terser-webpack-plugin'))],
}));

const GRIDS_NEW = path.join(__dirname, '../js/__internal/grids/new');

/**
* A read of `.value`, with its receiver chain so failures name the culprit.
*
* Assignment targets (`item.value = x`) are excluded, comparisons (`item.value === x`) kept.
* Without that exclusion Terser collapsing an if/else into a single ternary assignment looks
* exactly like a deleted read — `filtering/header_filter/legacy_header_filter.ts` does this and
* reported a false positive.
*/
const VALUE_READ = /[\w$.]*\.value\b(?!\s*=(?!=))/g;

/**
* `.tsx` is excluded: only `@babel/plugin-transform-typescript` is available here, with no JSX
* syntax plugin, so view files cannot be parsed. No known subscription lives in a view, but this
* is a real coverage gap — widen the filter if a JSX parser is ever added.
*/
function listModules(dir: string): string[] {
return fs.readdirSync(dir, { withFileTypes: true }).flatMap((entry) => {
const entryPath = path.join(dir, entry.name);

if (entry.isDirectory()) {
return listModules(entryPath);
}

const isTestSupport = /\.(test|mock|test_utils)\./.test(entry.name);
return entry.name.endsWith('.ts') && !isTestSupport ? [entryPath] : [];
});
}

const MODULES = listModules(GRIDS_NEW);

function stripTypes(absPath: string): string {
const result = transformSync(fs.readFileSync(absPath, 'utf8'), {
filename: absPath,
babelrc: false,
configFile: false,
plugins: ['@babel/plugin-transform-typescript'],
});

return result!.code!;
}

async function minify(absPath: string, pureGetters: boolean): Promise<string> {
const result = await terser.minify(stripTypes(absPath), {
module: true,
// Keeps property names and locals readable, so a lost read can be named in the failure.
// It is `compress`, not `mangle`, that deletes reads — this does not weaken the guard.
mangle: false,
compress: { pure_getters: pureGetters },
});

return result.code as string;
}

/** Multiset difference: reads present in `before` that `after` no longer has. */
function lostReads(before: string, after: string): string[] {
const remaining = after.match(VALUE_READ) ?? [];

return (before.match(VALUE_READ) ?? []).filter((read) => {
const at = remaining.indexOf(read);
if (at === -1) {
return true;
}
remaining.splice(at, 1);
return false;
});
}

describe('compress.pure_getters must not drop signal subscriptions (T1334012)', () => {
// Guards the harness itself: a module with no `.value` reads passes trivially, so a broken
// directory walk or an empty transform would turn the whole suite green.
it('finds a corpus of modules that actually read .value', async () => {
expect(MODULES.length).toBeGreaterThan(0);

const totalReads = (
await Promise.all(
MODULES.map(async (m) => ((await minify(m, false)).match(VALUE_READ) ?? []).length),
)
).reduce((total, n) => total + n, 0);

expect(totalReads).toBeGreaterThan(0);
});

it.each(MODULES.map((m) => [path.relative(GRIDS_NEW, m), m]))(
'%s',
async (_name: string, absPath: string) => {
const [before, after] = await Promise.all([minify(absPath, false), minify(absPath, true)]);

expect(lostReads(before, after)).toEqual([]);
},
);
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export {
computed,
effect,
signal,
track,
// eslint-disable-next-line spellcheck/spell-checker
untracked,
} from './reactive_primitives/index';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,5 @@ export function untracked<T>(fn: UntrackedFunction<T>): T {
// eslint-disable-next-line spellcheck/spell-checker
return Reactive.untracked(fn);
}

export { track } from '../../prod/reactive_primitives/index';
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export {
computed,
effect,
signal,
track,
// eslint-disable-next-line spellcheck/spell-checker
untracked,
} from './reactive_primitives/index';
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,23 @@ export function untracked<T>(fn: UntrackedFunction<T>): T {
// eslint-disable-next-line spellcheck/spell-checker
return SignalsCore.untracked(fn);
}

const trackSink: { last: unknown } = { last: undefined };

/**
* Registers `values` as dependencies of the enclosing computed/effect without using them.
*
* A standalone `signal.value;` read is load-bearing - it IS the subscription -
* but minifiers with `compress.pure_getters` delete it. A property assignment survives,
* because `pure_getters` makes no claim about setters (T1334012).
*
* The sink is cleared before returning: holding on to it would keep whatever was tracked last -
* often a whole item array - strongly referenced until the next `track` call anywhere.
*/
export function track(...values: unknown[]): void {
for (const value of values) {
trackSink.last = value;
}

trackSink.last = undefined;
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,5 +77,49 @@ describe('Reactive wrapper', () => {

untrackedDispose();
});

it('track subscribes the enclosing effect to its arguments', () => {
const first = ReactiveModule.signal(1);
const second = ReactiveModule.signal(1);

let effectRunCount = 0;

const dispose = ReactiveModule.effect(() => {
ReactiveModule.track(first.value, second.value);
effectRunCount += 1;
});

expect(effectRunCount).toBe(1);

first.value = 2;
expect(effectRunCount).toBe(2);

second.value = 2;
expect(effectRunCount).toBe(3);

dispose();

first.value = 3;
expect(effectRunCount).toBe(3);
});

it('track subscribes the enclosing computed to its arguments', () => {
const trackedSignal = ReactiveModule.signal(1);

let computedRunCount = 0;

const testComputed = ReactiveModule.computed(() => {
ReactiveModule.track(trackedSignal.value);
computedRunCount += 1;
return 'result';
});

expect(testComputed.value).toBe('result');
expect(computedRunCount).toBe(1);

trackedSignal.value = 2;
expect(testComputed.value).toBe('result');
expect(computedRunCount).toBe(2);
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { describe, expect, it } from '@jest/globals';

import { getContext } from '../di.test_utils';
import type { Options } from '../options';
import { OptionsControllerMock } from '../options_controller/options_controller.mock';
import { AccessibilityController } from './controller';

const setup = (config: Options = {}) => {
const context = getContext(config);

return {
optionsController: context.get(OptionsControllerMock),
accessibilityController: context.get(AccessibilityController),
};
};

describe('AccessibilityController', () => {
describe('componentStatus', () => {
// The status is announced only after the description changes, so the effect tracking the
// description is the only thing that can ever take the status out of its initial empty state.
it('should report the description after a column is hidden', () => {
const { optionsController, accessibilityController } = setup({
dataSource: [{ a: 'a_0', b: 'b_0' }],
columns: ['a', 'b'],
});

expect(accessibilityController.componentStatus.value).toBe('');

optionsController.option('columns', ['a']);

expect(accessibilityController.componentStatus.value)
.toBe(accessibilityController.componentDescription.peek());
expect(accessibilityController.componentStatus.value).not.toBe('');
});
});
});
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import messageLocalization from '@js/localization/message';
import { computed, effect, signal } from '@ts/core/state_manager/index';
import {
computed, effect, signal, track,
} from '@ts/core/state_manager/index';

import { ColumnsController } from '../columns_controller/columns_controller';
import { DataController } from '../data_controller/index';
Expand Down Expand Up @@ -38,8 +40,7 @@ export class AccessibilityController {

effect(() => {
// TODO: First Render refactor
// eslint-disable-next-line @typescript-eslint/no-unused-expressions
this.componentDescription.value;
track(this.componentDescription.value);

if (!firstRender) {
this.firstRender.value = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,26 @@ describe('Accessibility attributes', () => {
});
});

describe('Status container', () => {
it('should announce the description after it changes', () => {
const cardView = setup({
dataSource: [
{ A: 'A_0', B: 'B_0' },
{ A: 'A_1', B: 'B_1' },
],
columns: ['A', 'B'],
});

const statusContainer = rootQuerySelector(SELECTORS.statusContainer);
expect(statusContainer?.textContent).toBe('Card view with 2 cards. Each card has 2 fields');

cardView.columnOption('B', 'visible', false);
rerender();

expect(statusContainer?.textContent).toBe('Card view with 2 cards. Each card has 1 fields');
});
});

describe('Header panel', () => {
it('should be represented as menubar', () => {
setup({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,30 @@ describe('DataController', () => {
});
});

describe('remoteOperations', () => {
it('should reload the store when the option is changed at runtime', async () => {
const loadSpy = jest.fn(() => generateData(10));
const { optionsController, dataController } = setup({
dataSource: new CustomStore({
load: loadSpy,
}),
remoteOperations: {
filtering: false, sorting: false, paging: false,
},
});
await dataController.waitLoaded();

expect(loadSpy).toHaveBeenCalledTimes(1);

optionsController.option('remoteOperations', {
filtering: true, sorting: false, paging: false,
});
await dataController.waitLoaded();

expect(loadSpy).toHaveBeenCalledTimes(2);
});
});

describe('regressions', () => {
it('should work good with odata store', async () => {
const sendRequestSpy = jest.spyOn(ajax, 'sendRequest').mockImplementation((params: any) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import ArrayStore from '@js/common/data/array_store';
import { Deferred } from '@js/core/utils/deferred';
import { isDefined, isPlainObject } from '@js/core/utils/type';
import type { ReadonlySignal } from '@ts/core/state_manager/index';
import { computed, effect, signal } from '@ts/core/state_manager/index';
import {
computed, effect, signal, track,
} from '@ts/core/state_manager/index';
import { equalByValue } from '@ts/core/utils/m_common';
import type { PromiseWithResolvers } from '@ts/core/utils/promise';
import { createPromise } from '@ts/core/utils/promise';
Expand Down Expand Up @@ -230,8 +232,7 @@ export class DataController {

effect(
() => {
// eslint-disable-next-line @typescript-eslint/no-unused-expressions
this.normalizedRemoteOptions.value;
track(this.normalizedRemoteOptions.value);

if (this.dataSource.peek().isLoaded()) {
// eslint-disable-next-line @typescript-eslint/no-floating-promises
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,4 +148,41 @@ describe('ItemsController', () => {
expect(itemsController.items.peek()).toMatchSnapshot();
});
});

// `items` reads columns and highlight options non-reactively, so the subscriptions are separate.
// Each test changes only one of those signals, so a recompute proves the subscription.
describe('items dependency tracking', () => {
it('should recompute when a column becomes hidden', () => {
const { options, itemsController } = setup({
keyExpr: 'id',
dataSource: [{ id: 1, a: 'my a value', b: 'my b value' }],
columns: ['a', 'b'],
});

expect(itemsController.items.value[0].fields.map((field) => field.column.dataField))
.toEqual(['a', 'b']);

options.option('columns', ['a', { dataField: 'b', visible: false }]);

expect(itemsController.items.value[0].fields.map((field) => field.column.dataField))
.toEqual(['a']);
});

it('should recompute when highlighting options change', () => {
const { options, itemsController } = setup({
keyExpr: 'id',
dataSource: [{ id: 1, a: 'ABC' }],
columns: ['a'],
searchPanel: { text: 'abc', highlightCaseSensitive: false },
});

expect(itemsController.items.value[0].fields[0].highlightedText).not.toBeNull();

// Feeds highlightTextOptions only. The search filter is built from searchPanel.text,
// columns and searchVisibleColumnsOnly, so the data set does not change.
options.option('searchPanel.highlightCaseSensitive', true);

expect(itemsController.items.value[0].fields[0].highlightedText).toBeNull();
});
});
});
Loading
Loading