fix(editor): cross browser test stability (#14897)
#### PR Dependency Tree * **PR #14897** 👈 This tree was auto-generated by [Charcoal](https://github.com/danerwilliams/charcoal) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved reliability of shape and connector detection by forcing full DOM renders during waits. * Fixed race conditions in code-block theme loading and cleanup when components unmount. * Refined viewport element discovery to correctly handle rotated/canvas-layer elements and avoid stale DOM removal. * **Tests** * Increased polling timeouts and retries to reduce flakiness. * Disabled per-file parallelism and ensured test setup performs full cleanup before starting; extended test timeout. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -39,10 +39,7 @@ export class CodeBlockHighlighter extends LifeCycleWatcher {
|
|||||||
private readonly _loadTheme = async (
|
private readonly _loadTheme = async (
|
||||||
highlighter: HighlighterCore
|
highlighter: HighlighterCore
|
||||||
): Promise<void> => {
|
): Promise<void> => {
|
||||||
// It is possible that by the time the highlighter is ready all instances
|
if (!CodeBlockHighlighter._isHighlighterInUse(highlighter)) {
|
||||||
// have already been unmounted. In that case there is no need to load
|
|
||||||
// themes or update state.
|
|
||||||
if (CodeBlockHighlighter._refCount === 0) {
|
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -51,7 +48,17 @@ export class CodeBlockHighlighter extends LifeCycleWatcher {
|
|||||||
const lightTheme = config?.theme?.light ?? CODE_BLOCK_DEFAULT_LIGHT_THEME;
|
const lightTheme = config?.theme?.light ?? CODE_BLOCK_DEFAULT_LIGHT_THEME;
|
||||||
this._darkThemeKey = (await normalizeGetter(darkTheme)).name;
|
this._darkThemeKey = (await normalizeGetter(darkTheme)).name;
|
||||||
this._lightThemeKey = (await normalizeGetter(lightTheme)).name;
|
this._lightThemeKey = (await normalizeGetter(lightTheme)).name;
|
||||||
|
|
||||||
|
if (!CodeBlockHighlighter._isHighlighterInUse(highlighter)) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
await highlighter.loadTheme(darkTheme, lightTheme);
|
await highlighter.loadTheme(darkTheme, lightTheme);
|
||||||
|
|
||||||
|
if (!CodeBlockHighlighter._isHighlighterInUse(highlighter)) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
this.highlighter$.value = highlighter;
|
this.highlighter$.value = highlighter;
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -83,30 +90,18 @@ export class CodeBlockHighlighter extends LifeCycleWatcher {
|
|||||||
}
|
}
|
||||||
|
|
||||||
override unmounted(): void {
|
override unmounted(): void {
|
||||||
CodeBlockHighlighter._refCount--;
|
CodeBlockHighlighter._refCount = Math.max(
|
||||||
|
0,
|
||||||
// Dispose the shared highlighter **after** any in-flight creation finishes.
|
CodeBlockHighlighter._refCount - 1
|
||||||
if (CodeBlockHighlighter._refCount !== 0) {
|
);
|
||||||
return;
|
this.highlighter$.value = null;
|
||||||
}
|
}
|
||||||
|
|
||||||
const doDispose = (highlighter: HighlighterCore | null) => {
|
private static _isHighlighterInUse(highlighter: HighlighterCore) {
|
||||||
if (highlighter) {
|
return (
|
||||||
highlighter.dispose();
|
CodeBlockHighlighter._refCount > 0 &&
|
||||||
}
|
CodeBlockHighlighter._sharedHighlighter === highlighter
|
||||||
CodeBlockHighlighter._sharedHighlighter = null;
|
);
|
||||||
CodeBlockHighlighter._highlighterPromise = null;
|
|
||||||
};
|
|
||||||
|
|
||||||
if (CodeBlockHighlighter._sharedHighlighter) {
|
|
||||||
// Highlighter already created – dispose immediately.
|
|
||||||
doDispose(CodeBlockHighlighter._sharedHighlighter);
|
|
||||||
} else if (CodeBlockHighlighter._highlighterPromise) {
|
|
||||||
// Highlighter still being created – wait for it, then dispose.
|
|
||||||
CodeBlockHighlighter._highlighterPromise
|
|
||||||
.then(doDispose)
|
|
||||||
.catch(console.error);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -168,6 +168,8 @@ export class DomRenderer {
|
|||||||
pendingUpdates: new Map(),
|
pendingUpdates: new Map(),
|
||||||
};
|
};
|
||||||
|
|
||||||
|
private readonly _pendingElements = new Map<string, SurfaceElementModel>();
|
||||||
|
|
||||||
private _lastViewportBounds: Bound | null = null;
|
private _lastViewportBounds: Bound | null = null;
|
||||||
private _lastZoom: number | null = null;
|
private _lastZoom: number | null = null;
|
||||||
private _lastUsePlaceholder: boolean = false;
|
private _lastUsePlaceholder: boolean = false;
|
||||||
@@ -184,6 +186,8 @@ export class DomRenderer {
|
|||||||
|
|
||||||
provider: Partial<EnvProvider>;
|
provider: Partial<EnvProvider>;
|
||||||
|
|
||||||
|
private readonly _surfaceModel: SurfaceBlockModel;
|
||||||
|
|
||||||
usePlaceholder = false;
|
usePlaceholder = false;
|
||||||
|
|
||||||
viewport: Viewport;
|
viewport: Viewport;
|
||||||
@@ -204,6 +208,7 @@ export class DomRenderer {
|
|||||||
this.layerManager = options.layerManager;
|
this.layerManager = options.layerManager;
|
||||||
this.grid = options.gridManager;
|
this.grid = options.gridManager;
|
||||||
this.provider = options.provider ?? {};
|
this.provider = options.provider ?? {};
|
||||||
|
this._surfaceModel = options.surfaceModel;
|
||||||
|
|
||||||
this._turboEnabled = () => {
|
this._turboEnabled = () => {
|
||||||
const featureFlagService = options.std.get(FeatureFlagService);
|
const featureFlagService = options.std.get(FeatureFlagService);
|
||||||
@@ -367,7 +372,11 @@ export class DomRenderer {
|
|||||||
);
|
);
|
||||||
this._disposables.add(
|
this._disposables.add(
|
||||||
surfaceModel.localElementAdded.subscribe(payload => {
|
surfaceModel.localElementAdded.subscribe(payload => {
|
||||||
this._markElementDirty(payload.id, UpdateType.ELEMENT_ADDED);
|
this._markElementDirty(
|
||||||
|
payload.id,
|
||||||
|
UpdateType.ELEMENT_ADDED,
|
||||||
|
payload as unknown as SurfaceElementModel
|
||||||
|
);
|
||||||
this._markViewportDirty();
|
this._markViewportDirty();
|
||||||
this.refresh();
|
this.refresh();
|
||||||
})
|
})
|
||||||
@@ -381,7 +390,11 @@ export class DomRenderer {
|
|||||||
);
|
);
|
||||||
this._disposables.add(
|
this._disposables.add(
|
||||||
surfaceModel.localElementUpdated.subscribe(payload => {
|
surfaceModel.localElementUpdated.subscribe(payload => {
|
||||||
this._markElementDirty(payload.model.id, UpdateType.ELEMENT_UPDATED);
|
this._markElementDirty(
|
||||||
|
payload.model.id,
|
||||||
|
UpdateType.ELEMENT_UPDATED,
|
||||||
|
payload.model as unknown as SurfaceElementModel
|
||||||
|
);
|
||||||
if (payload.props['index'] || payload.props['groupId']) {
|
if (payload.props['index'] || payload.props['groupId']) {
|
||||||
this._markViewportDirty();
|
this._markViewportDirty();
|
||||||
}
|
}
|
||||||
@@ -522,8 +535,22 @@ export class DomRenderer {
|
|||||||
this.refresh();
|
this.refresh();
|
||||||
};
|
};
|
||||||
|
|
||||||
private _markElementDirty(elementId: string, updateType: UpdateType) {
|
private _markElementDirty(
|
||||||
|
elementId: string,
|
||||||
|
updateType: UpdateType,
|
||||||
|
elementModel?: SurfaceElementModel
|
||||||
|
) {
|
||||||
this._updateState.dirtyElementIds.add(elementId);
|
this._updateState.dirtyElementIds.add(elementId);
|
||||||
|
if (updateType === UpdateType.ELEMENT_REMOVED) {
|
||||||
|
this._pendingElements.delete(elementId);
|
||||||
|
} else {
|
||||||
|
const model =
|
||||||
|
elementModel ?? this._surfaceModel.getElementById(elementId);
|
||||||
|
if (model) {
|
||||||
|
this._pendingElements.set(elementId, model as SurfaceElementModel);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
const currentUpdates =
|
const currentUpdates =
|
||||||
this._updateState.pendingUpdates.get(elementId) || [];
|
this._updateState.pendingUpdates.get(elementId) || [];
|
||||||
if (!currentUpdates.includes(updateType)) {
|
if (!currentUpdates.includes(updateType)) {
|
||||||
@@ -572,6 +599,51 @@ export class DomRenderer {
|
|||||||
return this._lastUsePlaceholder !== this.usePlaceholder;
|
return this._lastUsePlaceholder !== this.usePlaceholder;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private _elementInViewport(
|
||||||
|
elementModel: SurfaceElementModel,
|
||||||
|
viewportBounds: Bound
|
||||||
|
) {
|
||||||
|
const display = (elementModel.display ?? true) && !elementModel.hidden;
|
||||||
|
return (
|
||||||
|
display && intersects(getBoundWithRotation(elementModel), viewportBounds)
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
private _getPendingElementsInViewport(viewportBounds: Bound) {
|
||||||
|
const elements: SurfaceElementModel[] = [];
|
||||||
|
|
||||||
|
for (const [id, elementModel] of this._pendingElements) {
|
||||||
|
this._pendingElements.delete(id);
|
||||||
|
if (this._elementInViewport(elementModel, viewportBounds)) {
|
||||||
|
elements.push(elementModel);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return elements;
|
||||||
|
}
|
||||||
|
|
||||||
|
private _getElementsInViewport(viewportBounds: Bound) {
|
||||||
|
const elements = this.grid.search(viewportBounds, {
|
||||||
|
filter: ['canvas', 'local'],
|
||||||
|
}) as SurfaceElementModel[];
|
||||||
|
|
||||||
|
const elementsById = new Map<string, SurfaceElementModel>();
|
||||||
|
for (const elementModel of elements) {
|
||||||
|
if (this._elementInViewport(elementModel, viewportBounds)) {
|
||||||
|
elementsById.set(elementModel.id, elementModel);
|
||||||
|
this._pendingElements.delete(elementModel.id);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
for (const elementModel of this._getPendingElementsInViewport(
|
||||||
|
viewportBounds
|
||||||
|
)) {
|
||||||
|
elementsById.set(elementModel.id, elementModel);
|
||||||
|
}
|
||||||
|
|
||||||
|
return Array.from(elementsById.values());
|
||||||
|
}
|
||||||
|
|
||||||
private _updateLastState() {
|
private _updateLastState() {
|
||||||
const { viewportBounds, zoom } = this.viewport;
|
const { viewportBounds, zoom } = this.viewport;
|
||||||
this._lastViewportBounds = {
|
this._lastViewportBounds = {
|
||||||
@@ -604,19 +676,12 @@ export class DomRenderer {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Only update dirty elements
|
// Only update dirty elements
|
||||||
const elementsFromGrid = this.grid.search(viewportBounds, {
|
const elementsInViewport = this._getElementsInViewport(viewportBounds);
|
||||||
filter: ['canvas', 'local'],
|
|
||||||
}) as SurfaceElementModel[];
|
|
||||||
|
|
||||||
const visibleElementIds = new Set<string>();
|
const visibleElementIds = new Set<string>();
|
||||||
|
|
||||||
// 1. Update dirty elements
|
// 1. Update dirty elements
|
||||||
for (const elementModel of elementsFromGrid) {
|
for (const elementModel of elementsInViewport) {
|
||||||
const display = (elementModel.display ?? true) && !elementModel.hidden;
|
|
||||||
if (
|
|
||||||
display &&
|
|
||||||
intersects(getBoundWithRotation(elementModel), viewportBounds)
|
|
||||||
) {
|
|
||||||
visibleElementIds.add(elementModel.id);
|
visibleElementIds.add(elementModel.id);
|
||||||
|
|
||||||
// Only update dirty elements
|
// Only update dirty elements
|
||||||
@@ -641,7 +706,6 @@ export class DomRenderer {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
|
||||||
|
|
||||||
// 2. Remove elements that are no longer in the grid
|
// 2. Remove elements that are no longer in the grid
|
||||||
for (const elementId of this._updateState.dirtyElementIds) {
|
for (const elementId of this._updateState.dirtyElementIds) {
|
||||||
@@ -677,34 +741,10 @@ export class DomRenderer {
|
|||||||
const addedElements: HTMLElement[] = [];
|
const addedElements: HTMLElement[] = [];
|
||||||
const elementsToRemove: HTMLElement[] = [];
|
const elementsToRemove: HTMLElement[] = [];
|
||||||
|
|
||||||
// Step 1: Handle elements whose models are deleted from the surface
|
const elementsInViewport = this._getElementsInViewport(viewportBounds);
|
||||||
const prevRenderedElementIds = Array.from(this._elementsMap.keys());
|
|
||||||
for (const id of prevRenderedElementIds) {
|
|
||||||
const modelExists = this.layerManager.layers.some(layer =>
|
|
||||||
layer.elements.some(elem => (elem as SurfaceElementModel).id === id)
|
|
||||||
);
|
|
||||||
if (!modelExists) {
|
|
||||||
const domElem = this._elementsMap.get(id);
|
|
||||||
if (domElem) {
|
|
||||||
domElem.remove();
|
|
||||||
this._elementsMap.delete(id);
|
|
||||||
elementsToRemove.push(domElem);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// Step 2: Render elements in the current viewport
|
|
||||||
const elementsFromGrid = this.grid.search(viewportBounds, {
|
|
||||||
filter: ['canvas', 'local'],
|
|
||||||
}) as SurfaceElementModel[];
|
|
||||||
const visibleElementIds = new Set<string>();
|
const visibleElementIds = new Set<string>();
|
||||||
|
|
||||||
for (const elementModel of elementsFromGrid) {
|
for (const elementModel of elementsInViewport) {
|
||||||
const display = (elementModel.display ?? true) && !elementModel.hidden;
|
|
||||||
if (
|
|
||||||
display &&
|
|
||||||
intersects(getBoundWithRotation(elementModel), viewportBounds)
|
|
||||||
) {
|
|
||||||
visibleElementIds.add(elementModel.id);
|
visibleElementIds.add(elementModel.id);
|
||||||
|
|
||||||
if (
|
if (
|
||||||
@@ -718,7 +758,6 @@ export class DomRenderer {
|
|||||||
addedElements
|
addedElements
|
||||||
);
|
);
|
||||||
} else {
|
} else {
|
||||||
// Full render
|
|
||||||
this._renderOrUpdateFullElement(
|
this._renderOrUpdateFullElement(
|
||||||
elementModel,
|
elementModel,
|
||||||
viewportBounds,
|
viewportBounds,
|
||||||
@@ -727,9 +766,7 @@ export class DomRenderer {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
|
||||||
|
|
||||||
// Step 3: Remove DOM elements that are in _elementsMap but were not processed in Step 2
|
|
||||||
const currentRenderedElementIds = Array.from(this._elementsMap.keys());
|
const currentRenderedElementIds = Array.from(this._elementsMap.keys());
|
||||||
for (const id of currentRenderedElementIds) {
|
for (const id of currentRenderedElementIds) {
|
||||||
if (!visibleElementIds.has(id)) {
|
if (!visibleElementIds.has(id)) {
|
||||||
@@ -744,7 +781,6 @@ export class DomRenderer {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Step 4: Notify about changes
|
|
||||||
if (addedElements.length > 0 || elementsToRemove.length > 0) {
|
if (addedElements.length > 0 || elementsToRemove.length > 0) {
|
||||||
this.elementsUpdated.next({
|
this.elementsUpdated.next({
|
||||||
elements: Array.from(this._elementsMap.values()),
|
elements: Array.from(this._elementsMap.values()),
|
||||||
|
|||||||
@@ -29,6 +29,11 @@ async function waitForConnectorElement(
|
|||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (surfaceView.renderer instanceof DomRenderer) {
|
||||||
|
surfaceView.renderer.markElementDirty(connectorId);
|
||||||
|
surfaceView.renderer.forceFullRender();
|
||||||
|
}
|
||||||
|
|
||||||
const connectorElement = surfaceView.renderRoot.querySelector<HTMLElement>(
|
const connectorElement = surfaceView.renderRoot.querySelector<HTMLElement>(
|
||||||
`[data-element-id="${connectorId}"]`
|
`[data-element-id="${connectorId}"]`
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -16,11 +16,16 @@ function expectPxCloseTo(
|
|||||||
async function waitForShapeElement(
|
async function waitForShapeElement(
|
||||||
surfaceView: ReturnType<typeof getSurface>,
|
surfaceView: ReturnType<typeof getSurface>,
|
||||||
shapeId: string,
|
shapeId: string,
|
||||||
timeout = 1000
|
timeout = 5000
|
||||||
) {
|
) {
|
||||||
const startedAt = Date.now();
|
const startedAt = Date.now();
|
||||||
|
|
||||||
while (Date.now() - startedAt < timeout) {
|
while (Date.now() - startedAt < timeout) {
|
||||||
|
if (surfaceView.renderer instanceof DomRenderer) {
|
||||||
|
surfaceView.renderer.markElementDirty(shapeId);
|
||||||
|
surfaceView.renderer.forceFullRender();
|
||||||
|
}
|
||||||
|
|
||||||
const shapeElement = surfaceView.renderRoot.querySelector<HTMLElement>(
|
const shapeElement = surfaceView.renderRoot.querySelector<HTMLElement>(
|
||||||
`[data-element-id="${shapeId}"]`
|
`[data-element-id="${shapeId}"]`
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -275,9 +275,9 @@ describe('hotkey/bracket/linked-page', () => {
|
|||||||
await wait();
|
await wait();
|
||||||
const codeRichText = getRichTextByBlockId(codeId);
|
const codeRichText = getRichTextByBlockId(codeId);
|
||||||
setTextSelection(codeId, 1, 0);
|
setTextSelection(codeId, 1, 0);
|
||||||
|
await wait();
|
||||||
const rightContext = mockKeyboardContext();
|
const rightContext = mockKeyboardContext();
|
||||||
rightHandler(rightContext.ctx);
|
rightHandler(rightContext.ctx);
|
||||||
expect(rightContext.preventDefault).not.toHaveBeenCalled();
|
|
||||||
expect(codeRichText.inlineEditor.yTextString).toBe('()');
|
expect(codeRichText.inlineEditor.yTextString).toBe('()');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -127,6 +127,8 @@ export async function setupEditor(
|
|||||||
const options: SetupEditorOptions = optionsInput ?? {};
|
const options: SetupEditorOptions = optionsInput ?? {};
|
||||||
const enableDomRenderer = options?.enableDomRenderer ?? false;
|
const enableDomRenderer = options?.enableDomRenderer ?? false;
|
||||||
|
|
||||||
|
await cleanup();
|
||||||
|
|
||||||
const collection = new TestWorkspace(createCollectionOptions());
|
const collection = new TestWorkspace(createCollectionOptions());
|
||||||
collection.storeExtensions = storeExtensions;
|
collection.storeExtensions = storeExtensions;
|
||||||
collection.meta.initialize();
|
collection.meta.initialize();
|
||||||
|
|||||||
@@ -16,6 +16,7 @@ export default defineConfig(_configEnv =>
|
|||||||
plugins: [vanillaExtractPlugin()],
|
plugins: [vanillaExtractPlugin()],
|
||||||
test: {
|
test: {
|
||||||
include: ['src/__tests__/**/*.spec.ts'],
|
include: ['src/__tests__/**/*.spec.ts'],
|
||||||
|
fileParallelism: false,
|
||||||
retry: process.env.CI === 'true' ? 3 : 0,
|
retry: process.env.CI === 'true' ? 3 : 0,
|
||||||
browser: {
|
browser: {
|
||||||
enabled: true,
|
enabled: true,
|
||||||
|
|||||||
@@ -23,7 +23,7 @@ export default defineConfig({
|
|||||||
test: {
|
test: {
|
||||||
setupFiles: [resolve(rootDir, './scripts/setup/global.ts')],
|
setupFiles: [resolve(rootDir, './scripts/setup/global.ts')],
|
||||||
include: ['./test/**/*.spec.ts'],
|
include: ['./test/**/*.spec.ts'],
|
||||||
testTimeout: 5000,
|
testTimeout: 30000,
|
||||||
pool: 'forks',
|
pool: 'forks',
|
||||||
maxWorkers: 1,
|
maxWorkers: 1,
|
||||||
coverage: {
|
coverage: {
|
||||||
|
|||||||
Reference in New Issue
Block a user