Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Content Model Cache improvement - Step 2: Prepare utility functions #2641

Merged
merged 30 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
7cc5a1b
Readonly types (3rd try
JiuqingSong May 13, 2024
3ff0779
Improve
JiuqingSong May 14, 2024
950a4ae
fix build
JiuqingSong May 14, 2024
2574fe2
Improve
JiuqingSong May 14, 2024
0180d29
Merge branch 'master' into u/jisong/readonlytype0513
JiuqingSong May 14, 2024
47b21f3
improve
JiuqingSong May 14, 2024
bd6b0cb
Improve
JiuqingSong May 14, 2024
cf3ea78
Add shallow mutable type
JiuqingSong May 14, 2024
e14e506
improve
JiuqingSong May 14, 2024
9a2d449
Improve
JiuqingSong May 14, 2024
719f5ed
improve
JiuqingSong May 14, 2024
caf7ef1
improve
JiuqingSong May 15, 2024
29495db
Merge branch 'master' into u/jisong/readonlytype0513
JiuqingSong May 16, 2024
0230a26
add test
JiuqingSong May 16, 2024
46fbe45
Readonly types step 2
JiuqingSong May 16, 2024
2afcf38
add test
JiuqingSong May 16, 2024
adb26a0
Improve
JiuqingSong May 16, 2024
e91e1a8
Merge branch 'master' into u/jisong/readonlytype0513
JiuqingSong May 17, 2024
6dbd78b
improve
JiuqingSong May 17, 2024
d0ee665
Merge branch 'u/jisong/readonlytype0513' into u/jisong/readonlytypes_…
JiuqingSong May 17, 2024
f659ca9
Merge branch 'master' into u/jisong/readonlytypes_step_2
JiuqingSong May 19, 2024
acceb1d
improve
JiuqingSong May 19, 2024
a69738f
Merge branch 'master' into u/jisong/readonlytype0513
JiuqingSong May 20, 2024
b4ab134
improve
JiuqingSong May 20, 2024
dea9f7f
Merge branch 'u/jisong/readonlytype0513' into u/jisong/readonlytypes_…
JiuqingSong May 20, 2024
f745036
Merge branch 'master' into u/jisong/readonlytypes_step_2
JiuqingSong May 20, 2024
806a1d3
Improve
JiuqingSong May 20, 2024
7a6b43f
fix test
JiuqingSong May 20, 2024
5e72761
improve
JiuqingSong May 20, 2024
e590215
Merge branch 'master' into u/jisong/readonlytypes_step_2
JiuqingSong May 21, 2024
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
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { cloneModel } from 'roosterjs-content-model-dom';
import { ContentModelPane, ContentModelPaneProps } from './ContentModelPane';
import { createRibbonPlugin, RibbonButton, RibbonPlugin } from '../../roosterjsReact/ribbon';
import { getRefreshButton } from './buttons/refreshButton';
Expand Down Expand Up @@ -70,8 +71,9 @@ export class ContentModelPanePlugin extends SidePanePluginImpl<
this.getComponent(component => {
this.editor.formatContentModel(
model => {
component.setContentModel(model);
setCurrentContentModel(model);
const clonedModel = cloneModel(model);
component.setContentModel(clonedModel);
setCurrentContentModel(clonedModel);

return false;
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
import * as React from 'react';
import { ContentModelWithDataset } from 'roosterjs-content-model-types';
import { FormatRenderer } from './utils/FormatRenderer';
import {
ContentModelWithDataset,
ShallowMutableContentModelWithDataset,
Copy link
Contributor

Choose a reason for hiding this comment

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

I understood the shallow type is something between the mutable and readonly, but I do not understand how to use it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, your understanding is right.

  1. when we create model object, they are all mutable.
  2. when we get a "connected" content model using formatContentModel, we pass the model to callback function as Readonly (later I will use ShallowMutableContentModelDocument instead, but for now you can treat it as readonly)
  3. for any part you want to change, call mutateBlock(), this will give you a shallow mutable type. That means, you can do any change to this object itself, including its direct property, format, or segments for a pragraph. If it is a block group, you can add/remove blocks its child blocks array.
  4. But for block group, the child blocks themselves are still readonly. That means when we call mutateBlock, it only clear cached element for this block itself (as well as cached Element for table rows if block is table, or list levels if block is list litem), but it will not clear cached element for child blocks (e.g. table cell under table row, or paragraph under table cell)
  5. To mutate child blocks, keep calling mutateBlock to child blocks
  6. There is no way to fully convert a readonly content model to fully mutable model now. And this should never happen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can read mutateBlock.ts (already checked in) for better understanding.

Copy link
Contributor

Choose a reason for hiding this comment

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

So that means that for shallow type are mutable, but their children are readonly. In case we need the children to be mutable, we call mutateBlock. Is my understanding correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that is right.

} from 'roosterjs-content-model-types';

const styles = require('./FormatView.scss');

export function MetadataView<T>(props: {
model: ContentModelWithDataset<T>;
renderers: FormatRenderer<T>[];
updater: (model: ContentModelWithDataset<T>, callback: (format: T | null) => T | null) => void;
updater: (
model: ShallowMutableContentModelWithDataset<T>,
callback: (format: T | null) => T | null
) => void;
}) {
const { model, renderers, updater } = props;
const metadata = React.useRef<T>(null);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,45 @@
import type { ContentModelBlockGroup, ContentModelListItem } from 'roosterjs-content-model-types';
import type {
ContentModelBlockGroup,
ContentModelListItem,
ReadonlyContentModelBlockGroup,
ReadonlyContentModelListItem,
} from 'roosterjs-content-model-types';

/**
* Search for all list items in the same thread as the current list item
* @param model The content model
* @param currentItem The current list item
* Search for all list items in the same thread as the current list item
*/
export function findListItemsInSameThread(
group: ContentModelBlockGroup,
currentItem: ContentModelListItem
): ContentModelListItem[] {
): ContentModelListItem[];

/**
* Search for all list items in the same thread as the current list item (Readonly)
* @param model The content model
* @param currentItem The current list item
*/
export function findListItemsInSameThread(
group: ReadonlyContentModelBlockGroup,
currentItem: ReadonlyContentModelListItem
): ReadonlyContentModelListItem[];

export function findListItemsInSameThread(
group: ReadonlyContentModelBlockGroup,
currentItem: ReadonlyContentModelListItem
): ReadonlyContentModelListItem[] {
const items: (ContentModelListItem | null)[] = [];

findListItems(group, items);

return filterListItems(items, currentItem);
}

function findListItems(group: ContentModelBlockGroup, result: (ContentModelListItem | null)[]) {
function findListItems(
group: ReadonlyContentModelBlockGroup,
result: (ReadonlyContentModelListItem | null)[]
) {
group.blocks.forEach(block => {
switch (block.blockType) {
case 'BlockGroup':
Expand Down Expand Up @@ -56,7 +79,7 @@ function findListItems(group: ContentModelBlockGroup, result: (ContentModelListI
});
}

function pushNullIfNecessary(result: (ContentModelListItem | null)[]) {
function pushNullIfNecessary(result: (ReadonlyContentModelListItem | null)[]) {
const last = result[result.length - 1];

if (!last || last !== null) {
Expand All @@ -65,10 +88,10 @@ function pushNullIfNecessary(result: (ContentModelListItem | null)[]) {
}

function filterListItems(
items: (ContentModelListItem | null)[],
currentItem: ContentModelListItem
items: (ReadonlyContentModelListItem | null)[],
currentItem: ReadonlyContentModelListItem
) {
const result: ContentModelListItem[] = [];
const result: ReadonlyContentModelListItem[] = [];
const currentIndex = items.indexOf(currentItem);
const levelLength = currentItem.levels.length;
const isOrderedList = currentItem.levels[levelLength - 1]?.listType == 'OL';
Expand Down Expand Up @@ -131,7 +154,7 @@ function filterListItems(
}

function areListTypesCompatible(
listItems: (ContentModelListItem | null)[],
listItems: (ReadonlyContentModelListItem | null)[],
currentIndex: number,
compareToIndex: number
): boolean {
Expand All @@ -146,7 +169,7 @@ function areListTypesCompatible(
);
}

function hasStartNumberOverride(item: ContentModelListItem, levelLength: number): boolean {
function hasStartNumberOverride(item: ReadonlyContentModelListItem, levelLength: number): boolean {
return item.levels
.slice(0, levelLength)
.some(level => level.format.startNumberOverride !== undefined);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import * as domToContentModel from 'roosterjs-content-model-dom/lib/domToModel/d
import * as updateCachedSelection from '../../../lib/corePlugin/cache/updateCachedSelection';
import { createContentModel } from '../../../lib/coreApi/createContentModel/createContentModel';
import {
ContentModelDocument,
DomToModelContext,
DomToModelOptionForCreateModel,
EditorCore,
Expand Down Expand Up @@ -362,7 +363,9 @@ describe('createContentModel and cache management', () => {
},
} as any;

cloneModelSpy = spyOn(cloneModel, 'cloneModel').and.callFake(x => x);
cloneModelSpy = spyOn(cloneModel, 'cloneModel').and.callFake(
x => x as ContentModelDocument
);

spyOn(domToContentModel, 'domToContentModel').and.returnValue(mockedNewModel);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,9 @@ describe('Editor', () => {
mockedModel
);

const cloneModelSpy = spyOn(cloneModel, 'cloneModel').and.callFake(x => x);
const cloneModelSpy = spyOn(cloneModel, 'cloneModel').and.callFake(
x => x as ContentModelDocument
);

const model = editor.getContentModelCopy('clean');
expect(model).toBe(mockedModel);
Expand Down
17 changes: 12 additions & 5 deletions packages/roosterjs-content-model-dom/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export { areSameFormats } from './domToModel/utils/areSameFormats';
export { isBlockElement } from './domToModel/utils/isBlockElement';
export { buildSelectionMarker } from './domToModel/utils/buildSelectionMarker';

export { updateMetadata, hasMetadata } from './modelApi/metadata/updateMetadata';
export { updateMetadata, getMetadata, hasMetadata } from './modelApi/metadata/updateMetadata';
export { isNodeOfType } from './domUtils/isNodeOfType';
export { isElementOfType } from './domUtils/isElementOfType';
export { getObjectKeys } from './domUtils/getObjectKeys';
Expand Down Expand Up @@ -130,10 +130,17 @@ export { retrieveModelFormatState } from './modelApi/editing/retrieveModelFormat
export { getListStyleTypeFromString } from './modelApi/editing/getListStyleTypeFromString';
export { getSegmentTextFormat } from './modelApi/editing/getSegmentTextFormat';

export { updateImageMetadata } from './modelApi/metadata/updateImageMetadata';
export { updateTableCellMetadata } from './modelApi/metadata/updateTableCellMetadata';
export { updateTableMetadata } from './modelApi/metadata/updateTableMetadata';
export { updateListMetadata, ListMetadataDefinition } from './modelApi/metadata/updateListMetadata';
export { updateImageMetadata, getImageMetadata } from './modelApi/metadata/updateImageMetadata';
export {
updateTableCellMetadata,
getTableCellMetadata,
} from './modelApi/metadata/updateTableCellMetadata';
export { updateTableMetadata, getTableMetadata } from './modelApi/metadata/updateTableMetadata';
export {
updateListMetadata,
getListMetadata,
ListMetadataDefinition,
} from './modelApi/metadata/updateListMetadata';

export { ChangeSource } from './constants/ChangeSource';
export { BulletListType } from './constants/BulletListType';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import type { ContentModelBlock, ContentModelBlockGroup } from 'roosterjs-content-model-types';
import type {
ContentModelBlock,
ShallowMutableContentModelBlockGroup,
} from 'roosterjs-content-model-types';

/**
* Add a given block to block group
* @param group The block group to add block into
* @param block The block to add
*/
export function addBlock(group: ContentModelBlockGroup, block: ContentModelBlock) {
export function addBlock(group: ShallowMutableContentModelBlockGroup, block: ContentModelBlock) {
group.blocks.push(block);
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import type {
ContentModelParagraph,
ContentModelSegment,
ContentModelSegmentFormat,
ShallowMutableContentModelBlockGroup,
ShallowMutableContentModelParagraph,
} from 'roosterjs-content-model-types';

/**
Expand All @@ -19,7 +21,28 @@ export function addSegment(
newSegment: ContentModelSegment,
blockFormat?: ContentModelBlockFormat,
segmentFormat?: ContentModelSegmentFormat
): ContentModelParagraph {
): ContentModelParagraph;

/**
* Add a given segment into a paragraph from its parent group. If the last block of the given group is not paragraph, create a new paragraph. (Shallow mutable)
* @param group The parent block group of the paragraph to add segment into
* @param newSegment The segment to add
* @param blockFormat The block format used for creating a new paragraph when need
* @returns The parent paragraph where the segment is added to
*/
export function addSegment(
group: ShallowMutableContentModelBlockGroup,
newSegment: ContentModelSegment,
blockFormat?: ContentModelBlockFormat,
segmentFormat?: ContentModelSegmentFormat
): ShallowMutableContentModelParagraph;

export function addSegment(
group: ShallowMutableContentModelBlockGroup,
newSegment: ContentModelSegment,
blockFormat?: ContentModelBlockFormat,
segmentFormat?: ContentModelSegmentFormat
): ShallowMutableContentModelParagraph {
const paragraph = ensureParagraph(group, blockFormat, segmentFormat);
const lastSegment = paragraph.segments[paragraph.segments.length - 1];

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import { addBlock } from './addBlock';
import { createParagraph } from '../creators/createParagraph';
import { mutateBlock } from './mutate';
import type {
ContentModelBlockFormat,
ContentModelBlockGroup,
ContentModelParagraph,
ContentModelSegmentFormat,
ShallowMutableContentModelBlockGroup,
ShallowMutableContentModelParagraph,
} from 'roosterjs-content-model-types';

/**
Expand All @@ -17,11 +20,29 @@ export function ensureParagraph(
group: ContentModelBlockGroup,
blockFormat?: ContentModelBlockFormat,
segmentFormat?: ContentModelSegmentFormat
): ContentModelParagraph {
): ContentModelParagraph;

/**
* @internal
* Ensure there is a Paragraph that can insert segments in a Content Model Block Group (Shallow mutable)
* @param group The parent block group of the target paragraph
* @param blockFormat Format of the paragraph. This is only used if we need to create a new paragraph
*/
export function ensureParagraph(
group: ShallowMutableContentModelBlockGroup,
blockFormat?: ContentModelBlockFormat,
segmentFormat?: ContentModelSegmentFormat
): ShallowMutableContentModelParagraph;

export function ensureParagraph(
group: ShallowMutableContentModelBlockGroup,
blockFormat?: ContentModelBlockFormat,
segmentFormat?: ContentModelSegmentFormat
): ShallowMutableContentModelParagraph {
const lastBlock = group.blocks[group.blocks.length - 1];

if (lastBlock?.blockType == 'Paragraph') {
return lastBlock;
return mutateBlock(lastBlock);
} else {
const paragraph = createParagraph(true, blockFormat, segmentFormat);
addBlock(group, paragraph);
Expand Down
Loading
Loading