[*] Chore: Fix all new lint warnings, plus manual exports clean-up #5979

etrepum · 2024-04-27T20:57:20Z

Description

Runs @lexical/eslint-plugin over the source to resolve all new warnings
Manually fix up names that were shadowed
Manually fix up invariants, flow types, and docs for renamed and deprecated public API
Manually fix up internal usage of newly deprecated APIs

Newly deprecated APIs (exports still available):

@lexical/link toggleLink - renamed to $toggleLink
@lexical/offset createChildrenArray - renamed to $createChildrenArray
@lexical/selection trimTextContentFromAnchor - renamed to $trimTextContentFromAnchor

Test Plan

Manually verify that the public API is stable in code review, either through no change or through a @deprecated alias
Manually verify that the flow types for any renamed or added public APIs are changed accordingly
All tests still pass

Review notes:

This is a really big PR if you read through the automated stuff. It could be reviewed in pieces:

Configuration change (first commit) - updateEditor is basically editor.update and should be considered a lexical provider
Automated npm run lint -- --fix (second commit) - this is completely automated and probably does not need close review
Manual fix-ups (almost everything after the second commit) - worth a close look
Doc improvements (last relevant commit) - adds {@link $newName} around deprecations for better typedoc ux, and removes unused markdown partial plugin
Flaky collab test mitigation? (not so relevant commit) adds some stuff to maybe help address collab tests that seem flaky. It seemed to help a bit locally, hard to tell whether it it's an solid improvement in CI or not until a lot of test runs have happened.

vercel · 2024-04-27T20:57:24Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
lexical	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 10, 2024 1:32pm
lexical-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 10, 2024 1:32pm

etrepum · 2024-05-05T21:40:39Z

packages/lexical-yjs/src/Utils.ts

-export function $createChildrenArray(
-  element: ElementNode,
-  nodeMap: null | NodeMap,
-): Array<NodeKey> {
-  const children = [];
-  let nodeKey = element.__first;
-  while (nodeKey !== null) {
-    const node =
-      nodeMap === null ? $getNodeByKey(nodeKey) : nodeMap.get(nodeKey);
-    if (node === null || node === undefined) {
-      invariant(false, 'createChildrenArray: node does not exist in nodeMap');
-    }
-    children.push(nodeKey);
-    nodeKey = node.__next;
-  }
-  return children;
-}
-/** @deprecated renamed to $createChildrenArray by @lexical/eslint-plugin rules-of-lexical */
-export const createChildrenArray = $createChildrenArray;
-


This is a dupe of the same function from @lexical/offset which is already a dependency of @lexical/yjs

etrepum · 2024-05-06T00:55:43Z

Pretty sure the test failure is just a flaky test, I can't repro locally on my mac and the tests succeeded earlier

This is the end of the output of my local CI=true npm run test-e2e-collab-ci-firefox

[1]   1) [firefox] › packages/lexical-playground/__tests__/regression/4697-repeated-table-selection.spec.mjs:21:3 › Regression test #4697 › repeated table selection results in table selection 
[1] 
[1]     Error: expect(received).toEqual(expected) // deep equality
[1] 
[1]     Expected: 1
[1]     Received: 2
[1] 
[1]        at packages/lexical-playground/__tests__/utils/index.mjs:267
[1] 
[1]       265 |     }
[1]       266 |     if (coordinates.anchor.y !== undefined) {
[1]     > 267 |       expect(_anchor.y).toEqual(coordinates.anchor.y);
[1]           |                         ^
[1]       268 |     }
[1]       269 |   }
[1]       270 |   if (coordinates.focus) {
[1] 
[1]         at assertTableSelectionCoordinates (/Users/bob/src/lexical/packages/lexical-playground/__tests__/utils/index.mjs:267:25)
[1]         at /Users/bob/src/lexical/packages/lexical-playground/__tests__/regression/4697-repeated-table-selection.spec.mjs:49:5
[1] 
[1]   Slow test file: [firefox] › packages/lexical-playground/__tests__/e2e/Markdown.spec.mjs (1.6m)
[1]   Slow test file: [firefox] › packages/lexical-playground/__tests__/e2e/Tables.spec.mjs (1.5m)
[1]   Slow test file: [firefox] › packages/lexical-playground/__tests__/e2e/List.spec.mjs (1.2m)
[1]   Slow test file: [firefox] › packages/lexical-playground/__tests__/e2e/Links.spec.mjs (1.1m)
[1]   Slow test file: [firefox] › packages/lexical-playground/__tests__/e2e/Navigation.spec.mjs (59.1s)
[1]   Consider splitting slow test files to speed up parallel execution
[1]   1 flaky
[1]     [firefox] › packages/lexical-playground/__tests__/regression/4697-repeated-table-selection.spec.mjs:21:3 › Regression test #4697 › repeated table selection results in table selection 
[1]   79 skipped
[1]   362 passed (4.1m)
[1] npm run test-e2e-collab-firefox exited with code 0

…ableElement -> $convertTableElement, convertTableRowElement -> $convertTableRowElement

etrepum · 2024-05-10T13:32:31Z

@ivailop7 as long as it happens in the morning, afternoon is no longer available. Just rebased.

ivailop7 · 2024-05-10T13:52:48Z

Which one should be the go to PR, this one or the #6078

etrepum · 2024-05-10T13:56:58Z

@ivailop7 they are the same but I don't know why the checks are not running and created another PR to try and figure it out. They are still not running, I don't know what to do here.

etrepum · 2024-05-10T13:58:38Z

@ivailop7 I don't think it's related to this changeset because it doesn't touch workflows and other PRs from today are affected (#6076, #6077 at least)

etrepum · 2024-05-10T14:03:40Z

@ivailop7 I have updated the links in the PR description to point to the current commits, so you could review it even though the tests are not running. They were all passing before the rebase. I will run them locally as a sanity check.

etrepum · 2024-05-10T14:16:46Z

Unit test suites are passing locally

$ git rev-parse HEAD
58a94e6e8d59343d194bf48d794558a5c6b8ae01
$ npm run test-unit
[…]
Test Suites: 50 passed, 50 total
Tests:       2 skipped, 1367 passed, 1369 total
Snapshots:   0 total
Time:        86.149 s
Ran all test suites.

etrepum · 2024-05-10T14:28:59Z

e2e suite runs with mac+chromium

$ npm run clean && CI=true npm run test-e2e-ci-chromium
  […]
  1 flaky
    [chromium] › packages/lexical-playground/__tests__/e2e/Links.spec.mjs:1959:3 › Links › Can handle pressing Enter inside a Link containing multiple TextNodes 
  19 skipped
  428 passed (2.0m)

I'll do a prod collab suite next but given the nature of these changes I don't expect that a rename could make a difference to any of that.

etrepum · 2024-05-10T14:36:18Z

@ivailop7 mac collab prod chromium is good to go as well, I have full confidence that there are no new regressions in this rebase.

$ npm run clean && CI=true npm run test-e2e-collab-prod-ci-chromium
[…]
[1]   2 flaky
[1]     [chromium] › packages/lexical-playground/__tests__/e2e/Links.spec.mjs:1959:3 › Links › Can handle pressing Enter inside a Link containing multiple TextNodes 
[1]     [chromium] › packages/lexical-playground/__tests__/e2e/Tables.spec.mjs:1267:3 › Tables › Grid selection: can select multiple cells and insert an image 
[1]   65 skipped
[1]   381 passed (2.8m)

etrepum · 2024-05-10T14:37:20Z

ah also ci-check passes

$ npm run ci-check
[…]
$ echo $?
0

ivailop7 · 2024-05-10T14:45:54Z

@etrepum ok, game time

… to facebook#5979)

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 27, 2024

vercel bot deployed to Preview – lexical April 27, 2024 20:58 View deployment

vercel bot deployed to Preview – lexical-playground April 27, 2024 21:00 View deployment

etrepum mentioned this pull request Apr 29, 2024

feat(@lexical/eslint-plugin): new package with eslint rules for lexical #5908

Merged

etrepum force-pushed the lexical-eslint-plugin-autofixes branch from 095502e to 92f416e Compare May 5, 2024 20:24

vercel bot deployed to Preview – lexical-playground May 5, 2024 20:25 View deployment

vercel bot deployed to Preview – lexical May 5, 2024 20:26 View deployment

etrepum changed the title ~~[NO MERGE] Lexical eslint plugin autofixes~~ [NO MERGE] Lexical eslint plugin lint fixes May 5, 2024

etrepum force-pushed the lexical-eslint-plugin-autofixes branch from 92f416e to c9613e7 Compare May 5, 2024 21:13

vercel bot deployed to Preview – lexical May 5, 2024 21:14 View deployment

vercel bot deployed to Preview – lexical-playground May 5, 2024 21:15 View deployment

vercel bot deployed to Preview – lexical May 5, 2024 21:21 View deployment

vercel bot deployed to Preview – lexical-playground May 5, 2024 21:21 View deployment

etrepum changed the title ~~[NO MERGE] Lexical eslint plugin lint fixes~~ [NO MERGE] [all] Chore: Fix all new lint warnings, plus manual exports clean-up May 5, 2024

etrepum changed the title ~~[NO MERGE] [all] Chore: Fix all new lint warnings, plus manual exports clean-up~~ [*] Chore: Fix all new lint warnings, plus manual exports clean-up May 5, 2024

etrepum marked this pull request as ready for review May 5, 2024 21:28

etrepum requested review from zurfyx, fantactuka, acywatson, Fetz and ivailop7 as code owners May 5, 2024 21:29

etrepum commented May 5, 2024

View reviewed changes

vercel bot deployed to Preview – lexical-playground May 5, 2024 22:26 View deployment

vercel bot deployed to Preview – lexical May 5, 2024 22:26 View deployment

vercel bot deployed to Preview – lexical May 5, 2024 22:51 View deployment

vercel bot deployed to Preview – lexical-playground May 5, 2024 22:52 View deployment

vercel bot deployed to Preview – lexical May 6, 2024 15:48 View deployment

vercel bot deployed to Preview – lexical-playground May 6, 2024 15:49 View deployment

etrepum added 9 commits May 10, 2024 06:26

mergePrevious -> $mergePrevious

b5ba4ca

trimTextContentFromAnchor -> $trimTextContentFromAnchor

7724d8d

setAnchorPoint -> $setAnchorPoint, setFocusPoint -> $setFocusPoint

ccf296a

convertTableCellNodeElement -> $convertTableCellNodeElement, convertT…

1a799b1

…ableElement -> $convertTableElement, convertTableRowElement -> $convertTableRowElement

getOrInitCollabNodeFromSharedType -> $getOrInitCollabNodeFromSharedType

4828d9e

syncLocalCursorPosition -> $syncLocalCursorPosition

da0c7ba

newline in flow file

1c60e7b

add {@link $newName} to deprecation comments

d5b6ea8

attempt to reduce a bit of test flakiness?

58a94e6

etrepum force-pushed the lexical-eslint-plugin-autofixes branch from 58321ab to 58a94e6 Compare May 10, 2024 13:30

vercel bot deployed to Preview – lexical-playground May 10, 2024 13:31 View deployment

vercel bot deployed to Preview – lexical May 10, 2024 13:32 View deployment

etrepum closed this May 10, 2024

etrepum reopened this May 10, 2024

ivailop7 approved these changes May 10, 2024

View reviewed changes

ivailop7 merged commit 081188c into facebook:main May 10, 2024
4 checks passed

etrepum added a commit to etrepum/lexical that referenced this pull request May 10, 2024

Chore: Fix deprecated usage of toggleLink with $toggleLink (follow up…

d575600

… to facebook#5979)

etrepum mentioned this pull request May 10, 2024

[lexical-link][lexical-react] Chore: Fix deprecated usage of toggleLink with $toggleLink #6081

Merged

etrepum added a commit to etrepum/lexical that referenced this pull request May 10, 2024

Chore: Fix deprecated usage of toggleLink with $toggleLink (follow up…

d719289

… to facebook#5979)

etrepum deleted the lexical-eslint-plugin-autofixes branch May 11, 2024 06:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[*] Chore: Fix all new lint warnings, plus manual exports clean-up #5979

[*] Chore: Fix all new lint warnings, plus manual exports clean-up #5979

etrepum commented Apr 27, 2024 •

edited

vercel bot commented Apr 27, 2024 •

edited

etrepum May 5, 2024

etrepum commented May 6, 2024 •

edited

etrepum commented May 10, 2024

ivailop7 commented May 10, 2024

etrepum commented May 10, 2024

etrepum commented May 10, 2024

etrepum commented May 10, 2024

etrepum commented May 10, 2024

etrepum commented May 10, 2024

etrepum commented May 10, 2024

etrepum commented May 10, 2024

ivailop7 commented May 10, 2024

[*] Chore: Fix all new lint warnings, plus manual exports clean-up #5979

[*] Chore: Fix all new lint warnings, plus manual exports clean-up #5979

Conversation

etrepum commented Apr 27, 2024 • edited

Description

Test Plan

Review notes:

vercel bot commented Apr 27, 2024 • edited

etrepum May 5, 2024

Choose a reason for hiding this comment

etrepum commented May 6, 2024 • edited

etrepum commented May 10, 2024

ivailop7 commented May 10, 2024

etrepum commented May 10, 2024

etrepum commented May 10, 2024

etrepum commented May 10, 2024

etrepum commented May 10, 2024

etrepum commented May 10, 2024

etrepum commented May 10, 2024

etrepum commented May 10, 2024

ivailop7 commented May 10, 2024

etrepum commented Apr 27, 2024 •

edited

vercel bot commented Apr 27, 2024 •

edited

etrepum commented May 6, 2024 •

edited