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 1: Introduce Readonly types and mutate utility #2629

Merged
merged 20 commits into from
May 20, 2024

Conversation

JiuqingSong
Copy link
Contributor

@JiuqingSong JiuqingSong commented May 13, 2024

This is the first step of Content Model cache improvement.

Step 1: Introduce read only types and mutate utility.

In order to make Content Model cache to be better used, we need to make sure:

  1. cached element is preserved when the related model is not changed
  2. cached element must be deleted from Content model if its related model is changed

To achieve these goals, we need to:

  1. Decide where to put the cached element. For now we will put it in all block and block groups
  2. Do not delete cached element (not included in this change)
  3. Add a utility function to delete cached element (included)
  4. Call this utility function before we modify something (not included)
  5. Let the build fail if we are about to change a model that still have cached element

To do 5, the easiest way is to use "reaodnly" keyword. However, due to microsoft/TypeScript#13347, assigning readonly type to mutable type won't trigger any build time error. So we need to introduce new readonly types, which will prevent modifying its members. And for these new types we also want to:

  1. When assign a mutable object to readonly type, it can success.
  2. When assign a readonly object to mutable type, it should fail

To do this, I introduced two marker interfaces: MutableMarker and ReadonlyMarker.

  • MutableMarker and all its derived types can be assigned to ReadonlyMarker and its derived types
  • ReadonlyMarker and all its derived types cannot be assigned to MutableMarker and its derived types

Each marker has a dummy property with different types. This dummy property is declared as optional so existing creators can still work. And I declare them as type "never[]" and "ReadonlyArray". So that we can assign array to readonly array, but not the other way. And declaring its base type as "never" so we can never really assign value to it. It is just a build time marker, no run time impact.

interface MyTypeCommon { 
...
}

let mutable: MutableMarker & MyTypeCommon = { ... };
let readonly: ReaadonlyMarker & MyTypeCommon = { ... };

readonly = mutable; // Ok
mutable = readonly; // Error

Then we can add these markers to all existing type and new readonly types.

For BlockGroups, we only want to mutate its single level properties, but not its child blocks. To mutate child blocks, call mutateBlock() to the child block that you need to mutate. So we need a 3rd marker: ShallowMutabeMarker. This is in between with MutableMarker and ReadonlyMarker. We can assign values in the following way, but not the other way:

MutableMarker => ShallowMutableMarker => ReadonlyMarker

  • When mutate a block, it becomes a fully mutable object
  • When mutate a paragraph block, all its segments are also mutable
  • When mutate a block group, this object itself is mutable, we can change its format, dataset (if any), decorator (if any), and add/remove block, but we cannot modify its existing child block
  • When mutate a paragraph with General Segment, since General Segment is also a block group, it will become shallow mutable, its child blocks are not mutable

To mutate a block, do

const block: ReadonlyContentModelBlock = { ... };
const mutableBlock = mutateBlock(block);

block.format = {... }; // Error
mutableBlock.format = { ... }; // Now you can modify this block, mutableBlock and block are actually the same object

To mutate a segment, need to know is owner paragraph as well

const paragraph: ReadonlyContentModelParagraph = { ... }
const segment = paragraph.segments[0];

segment.format = {...}; // Error, segment is readonly

mutateSegment(paragraph, segment, mutableSegment => {
   mutableSegment.format = { ... }; // OK
});

Need to make sure segment really belongs to this paragraph. Otherwise the callback won't be called

To mutate a bunch of segments, use mutateSegments:

const [mutableParagraph, mutableSegments] = mutateSegments(paragraph, [seg1, seg2, ... ] );

mutableSegments.forEach(seg => {
   seg.format = {...}; // ok
});

Only segments belong to this paragraph will be returned.

When mutate a table, all its rows will become mutable, but all cells are not.
When mutate a list, all its levels will become mutable, but its child blocks are not.

@JiuqingSong JiuqingSong changed the title Readonly types (3rd try Readonly types (3rd try) May 13, 2024
@JiuqingSong JiuqingSong changed the title Readonly types (3rd try) Content Model Cache improvement - Step 1: Introduce Readonly types and mutate utility May 16, 2024
@JiuqingSong JiuqingSong marked this pull request as ready for review May 16, 2024 17:57
Copy link
Contributor

@BryanValverdeU BryanValverdeU left a comment

Choose a reason for hiding this comment

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

Looks good to me. Clever solution.
Just curious. When the Typescript issue is resolved will this continue working?

@JiuqingSong
Copy link
Contributor Author

Looks good to me. Clever solution. Just curious. When the Typescript issue is resolved will this continue working?

According to their design, they will add a new build option so without the option our code will still work.

@JiuqingSong JiuqingSong merged commit 8ec51b3 into master May 20, 2024
7 checks passed
@JiuqingSong JiuqingSong deleted the u/jisong/readonlytype0513 branch May 20, 2024 21:03
Andres-CT98 added a commit that referenced this pull request May 24, 2024
* code suport

* markdown

* adjust

* demo site

* WIP

* fix code pen

* fix demo site

* Handle Tab key on whole table selection or last cell on Edit Plugin (#2536)

* Implement indent whole table

* implement tab on last cell new row

* add Table selection and single cell handling

* export clearSelectedCells

* undefined check, simplify

* add tests

* add test for list in table edge case

* Fix seelection on void element (#2551)

* port hyphen

* Implement movement with Tab key inside Table (#2529)

* implement tab movement, tests

* remove unused variable

* fix name, normalisation, add test

* fix wrong parethesis

* restore normalizePos

* fix tests

* add formatTextSegmentBeforeSelectionMarker

* Improve backspace on list (#2555)

* fix selection with ctrl+a

* refactor code using formatTextSegmentBeforeSelectionMarkerTest

* remove getLinkSegment

* Set default format in demo site (#2559)

* clean demo site

* clean

* Enable selecting image when the only element in the range is an Image (#2554)

* init

* Address comment

* Reuse isReverted from Range Selection

* Fix build

* Fix build

* Unselect image when Up or Down, or it remains selected

* remove unneeded changes and improve name of tests

* Update

---------

Co-authored-by: Julia Roldi <87443959+juliaroldi@users.noreply.github.com>

* fix markdown plugin

* Port Hyperlink plugin (#2560)

* Port Hyperlink plugin

* improve

---------

Co-authored-by: Bryan Valverde U <bvalverde@microsoft.com>

* export formatTextSegmentBeforeSelectionMarker

* fix build

* Prevent ScrollTop to be lost when the focus is done to the editor (#2564)

* init

* init

---------

Co-authored-by: Julia Roldi <87443959+juliaroldi@users.noreply.github.com>

* Fix 265959: extra empty line generated when get plain text (#2566)

Co-authored-by: Bryan Valverde U <bvalverde@microsoft.com>

* Fix #2500 Hyperlink misses color (#2570)

* Fix #2500

* fix test

* mac shortcuts

* fix test

* restore selection (#2577)

* fix: on webkit-based applications, when the selection is empty, focus will cause the window to scroll to the top (#2571)

Co-authored-by: Julia Roldi <87443959+juliaroldi@users.noreply.github.com>
Co-authored-by: Jiuqing Song <jisong@microsoft.com>

* Port PickerPlugin (#2569)

* Port PickerPlugin

* fix buid

* Improve

* fix build

* Improve

* Improve

* add test

* Improve

* adjust image

* fix test

* fixes

* remove

* Preserve reverted selection info in Content Model (#2580)

* Preserve reverted selection info in Content Model

* improve

* fix empty text

* trigger Events

* Allow Shift+Delete to Cut (#2585)

* Allow Shift+Delete to Cut

* improve

* change source

* fix build

* Fix #2584: Safari context menu event causes selection to be expanded (#2588)

* Fix #2584: Safari context menu event causes selection to be expanded

* fix test

* Port AnnouncePlugin step 1: refactor list number code (#2589)

* test

* refactor

* fix demo site

* Select image after inserting it (#2593)

* Focus image after insertion

* Revert "Focus image after insertion"

This reverts commit 887c9e5.

* Use setSelection

* Port AnnouncePlugin Step 2: Add announce core API (#2591)

* Port AnnouncePlugin step 1: refactor list number code

* Port AnnouncePlugin step 2

---------

Co-authored-by: Bryan Valverde U <bvalverde@microsoft.com>

* WIP

* custom replace

* Fix table mover selector (#2596)

* fix edge case tables and trigger contentchanged

* revert event

* trigger autolink

* wip

* Port AnnouncePlugin step 3: Add announce features for list and table (#2592)

* Port AnnouncePlugin step 1: refactor list number code

* Port AnnouncePlugin step 2

* Port AnnouncePlugin ste 3

* add test

* Fix #2575 Entity delimiter cursor moving (#2581)

* Preserve reverted selection info in Content Model

* Entity delimiter cursor moving (#2575)

* improve

* Add test

* add test

* Allow customizability for table editors (#2603)

* init

* Simplify type callback

* Also allow using Ctrl-Shift-Z on Windows (#2607)

* Also allow using Ctrl-Shift-Z on Windows

* Fix failing test

* Fix broken test

* Fix failing test

* Keep and deprecate old ShortcutRedoMacOS

* Remove test not needed anymore

* Wip

* add span

* fix test

* changes

* remove code

* fixes

* fixes

* Hide watermark when input with IME (#2611)

* Updated watermark property access modifier to protected (#2614)

* Move Content Model type files into contentModel folder (#2602)

* Fix 262779 (#2600)

* Add undo snapshot when mouse down if there is new content after last undo snapshot (#2604)

* Fix #2601 allow customization when convert from content model to plain text (#2605)

* Fix #2601 allow customization when convert from content model to plain text

* Improve

* fix test

* fix build

* Scroll caret into view when call formatContentModel (#2617)

* Perf step 0: Do not allow getting connected model (#2615)

* Perf: Do not allow getting connected model

* fix build

* fix build

* debug firefox options (#2620)

* Updated isModelEmptyFast to consider changed indentation as not empty string (#2625)

* Updated isModelEmptyFast

* Tests fix

* Add parameter to Disable table edit features (#2624)

* disableFeatures, renaming

* Export Table Feature Ids

* Modify and create tests

* move Id to respective feature, add tests, cleanup

* export feature names

* TableEditFeatureName

* Implement Table Movement (#2599)

* isNodeEditor

* domHelper test

* implementation and test fix

* fix undefined

* isNodeEditor

* implement live focus, restore initial selection on cancel

* broswer drag and drop attempt

* use getNodePositionFromEvent, add move cursor while dragging

* restore class

* fix import

* restore DOMHelper

* remove unneeded snapshots and checks, add copy feature

* cleanup

* fix merge

* cursor change on copy,  remove onEditorCreated

* fix disposal issue

* removed unused forEach,

* export formatInsertPointWithContentModel

* use formatInsertPointWithContentModel for table movement

* reorder function

* Add tests for table movement

* optimisations and exports for testing

* restore final table selection after move

* added test

* fix and add more tests

* comments, remove copy feature, fix tests

* fix test

* fix test

* implement disable movement

* Do not apply Table selection if table is not editable (#2628)

* Fix #2633 `scrollCaretIntoView` causes unexpected scroll (#2634)

* Fix #2633

* add comment

* Added tableCellSelectionBackgroundColor option (#2640)

* Added tableCellSelectionBackgroundColor option

* Fixed a test

* Tests fix

* Tests fix

* Add dark color handling for table and image selection (#2647)

* Added tableCellSelectionBackgroundColor option

* Fixed a test

* Tests fix

* Tests fix

* Add dark color check

* Add more tests

* Updated login and tests

* Functionality update

* Update checks

* Added transparent options test

* Code review fixes

* Fix cursor jump issue when applying Gboard suggestions (#2638)

* fix cursor issue

* resolve comment

---------

Co-authored-by: Jiuqing Song <jisong@microsoft.com>

* Content Model Cache improvement - Step 1: Introduce Readonly types and mutate utility (#2629)

* Readonly types (3rd try

* Improve

* fix build

* Improve

* improve

* Improve

* Add shallow mutable type

* improve

* Improve

* improve

* improve

* add test

* improve

* improve

* improve

* test

* test

* link preview

* remove auto format plugin code

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

* Readonly types (3rd try

* Improve

* fix build

* Improve

* improve

* Improve

* Add shallow mutable type

* improve

* Improve

* improve

* improve

* add test

* Readonly types step 2

* add test

* Improve

* improve

* improve

* improve

* Improve

* fix test

* improve

* Content Model Cache improvement - Step 3: Let creators accept readonly types (#2642)

* Readonly types (3rd try

* Improve

* fix build

* Improve

* improve

* Improve

* Add shallow mutable type

* improve

* Improve

* improve

* improve

* add test

* Readonly types step 3

* improve

* improve

* improve

* Content Model Cache improvement - Step 4: Port "readonly" functions (#2643)

* Readonly types (3rd try

* Improve

* fix build

* Improve

* improve

* Improve

* Add shallow mutable type

* improve

* Improve

* improve

* improve

* add test

* Readonly types step 2

* Readonly types step 3

* Readonly type step 4

* add test

* Improve

* improve

* improve

* improve

* improve

* Improve

* improve

* fix test

* improve

* Fix Table first column (#2652)

* create Table Options button for demo

* fix setFirstColumnFormat

* fix dependency

* restore background color check for first column

* remove logs, move TableOptionsMenuItemStringKey, isHeader

* remove import

* Content Model Cache improvement - Step 5: Port roosterjs-content-model-dom package (#2648)

* Readonly types (3rd try

* Improve

* fix build

* Improve

* improve

* Improve

* Add shallow mutable type

* improve

* Improve

* improve

* improve

* add test

* Readonly types step 2

* Readonly types step 3

* Readonly type step 4

* add test

* Improve

* improve

* improve

* Readonly types step 5: dom package

* add change

* improve

* improve

* Improve

* improve

* fix test

* Improve

* fix build

* improve

* Optimise content model table fetching for Table Edit Plugins  (#2656)

* optimise cmTable fetching

* create getCMTableFromTable

* Fix normalisation and First Column issues (#2657)

* optimise cmTable fetching

* First Column not apply to very first cell

* fix normalisation

* fix first cell color for First Column

* fix and add test

* fix test

* fix tests

* fix test and hasHeaderRow case

* reorder rows

* bump

* bump

---------

Co-authored-by: Júlia Roldi <juliaroldi@microsoft.com>
Co-authored-by: Julia Roldi <87443959+juliaroldi@users.noreply.github.com>
Co-authored-by: Jiuqing Song <jisong@microsoft.com>
Co-authored-by: Bryan Valverde U <bvalverde@microsoft.com>
Co-authored-by: Rain-Zheng <67583056+Rain-Zheng@users.noreply.github.com>
Co-authored-by: 庄黛淳华 <miku1958@users.noreply.github.com>
Co-authored-by: florian-msft <87671048+florian-msft@users.noreply.github.com>
Co-authored-by: vhuseinova-msft <98852890+vhuseinova-msft@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants