Skip to content

[lexical-playground] Bug Fix: importing an unparseable data-lexical-datetime no longer breaks serialization - #9047

Merged
etrepum merged 3 commits into
facebook:mainfrom
LeSingh1:fix/datetime-import-invalid-date
Aug 9, 2026
Merged

[lexical-playground] Bug Fix: importing an unparseable data-lexical-datetime no longer breaks serialization#9047
etrepum merged 3 commits into
facebook:mainfrom
LeSingh1:fix/datetime-import-invalid-date

Conversation

@LeSingh1

@LeSingh1 LeSingh1 commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Description

DateTimeRule builds a node from the attribute without checking that it parsed:

const DateTimeRule = defineImportRule({
  $import: (ctx, el) => {
    const dateTimeValue = el.getAttribute('data-lexical-datetime')!;
    const node = $createDateTimeNode(new Date(Date.parse(dateTimeValue)));   // NaN -> Invalid Date
    const [firstChild] = ctx.$importChildren(el);
    ...
    return [node];
  },
  match: sel.tag('span').attr('data-lexical-datetime', true),

GoogleDocsDateRule, twenty lines below in the same file and matching the same kind of pasted markup, does exactly the check that is missing:

if (isNaN(parsedDate)) {
  return $next();
}

When Date.parse fails, the rule still claims the element and returns the node, so $next() is never reached and the element's own text is discarded. The node then holds an Invalid Date, and its state config unparses with toISOString():

const dateTimeState = createState('dateTime', {
  parse: v => new Date(v as string),
  unparse: v => v.toISOString(),
});

Date.prototype.toISOString throws RangeError: Invalid time value on an Invalid Date, and LexicalNodeState.toJSON calls unparse for any value that differs from the default — which an Invalid Date object always does.

Measured on upstream/main, pasting <span data-lexical-datetime="not a date">some text</span>:

INVALID       text= "Invalid Date NaN:NaN"   toJSON THREW: RangeError Invalid time value
GDOCS-INVALID text= "gd text"                toJSON: OK
VALID         text= "Thu May 28 2026 17:00"  toJSON: OK

So one bad attribute in pasted HTML both replaces the visible text with Invalid Date NaN:NaN and makes the whole editor state unserializable: every later editorState.toJSON() throws, which is what the playground's autosave, the shareable-document URL, collab and the debug tree view all call. The document cannot be saved again until that node is deleted. The sibling rule shows the intended handling — fall through and keep the element as ordinary content.

This adds the same guard. $next was already part of the rule signature, it simply was not used here. No API or serialization change.

Test plan

New unit test packages/lexical-playground/__tests__/unit/DateTimeImport.test.ts. Two of the three cases are controls that pass before and after: a parseable date must still import, and the Google Docs rule's existing behaviour is asserted alongside so the two rules are pinned to the same contract.

Before

 ❯ packages/lexical-playground/__tests__/unit/DateTimeImport.test.ts (3 tests | 1 failed)
   × keeps the element content when the date cannot be parsed
     AssertionError: expected true to be false // Object.is equality
   ✓ still imports a parseable date
   ✓ matches how the Google Docs rule already handles an unparseable date

 Test Files  1 failed (1)
      Tests  1 failed | 2 passed (3)

After

 Test Files  1 passed (1)
      Tests  3 passed (3)

Package suite is unchanged:

$ npx vitest run packages/lexical-playground
 Test Files  19 passed (19)
      Tests  277 passed (277)

…atetime no longer breaks serialization

## Description

`DateTimeRule` builds a node from the attribute without checking that it parsed:

```ts
const DateTimeRule = defineImportRule({
  $import: (ctx, el) => {
    const dateTimeValue = el.getAttribute('data-lexical-datetime')!;
    const node = $createDateTimeNode(new Date(Date.parse(dateTimeValue)));   // NaN -> Invalid Date
    const [firstChild] = ctx.$importChildren(el);
    ...
    return [node];
  },
  match: sel.tag('span').attr('data-lexical-datetime', true),
```

`GoogleDocsDateRule`, twenty lines below in the same file and matching the same kind of pasted markup, does exactly the check that is missing:

```ts
if (isNaN(parsedDate)) {
  return $next();
}
```

When `Date.parse` fails, the rule still claims the element and returns the node, so `$next()` is never reached and the element's own text is discarded. The node then holds an `Invalid Date`, and its state config unparses with `toISOString()`:

```ts
const dateTimeState = createState('dateTime', {
  parse: v => new Date(v as string),
  unparse: v => v.toISOString(),
});
```

`Date.prototype.toISOString` throws `RangeError: Invalid time value` on an Invalid Date, and `LexicalNodeState.toJSON` calls `unparse` for any value that differs from the default — which an `Invalid Date` object always does.

Measured on `upstream/main`, pasting `<span data-lexical-datetime="not a date">some text</span>`:

```
INVALID       text= "Invalid Date NaN:NaN"   toJSON THREW: RangeError Invalid time value
GDOCS-INVALID text= "gd text"                toJSON: OK
VALID         text= "Thu May 28 2026 17:00"  toJSON: OK
```

So one bad attribute in pasted HTML both replaces the visible text with `Invalid Date NaN:NaN` and makes the whole editor state unserializable: every later `editorState.toJSON()` throws, which is what the playground's autosave, the shareable-document URL, collab and the debug tree view all call. The document cannot be saved again until that node is deleted. The sibling rule shows the intended handling — fall through and keep the element as ordinary content.

This adds the same guard. `$next` was already part of the rule signature, it simply was not used here. No API or serialization change.

## Test plan

New unit test `packages/lexical-playground/__tests__/unit/DateTimeImport.test.ts`. Two of the three cases are controls that pass before and after: a parseable date must still import, and the Google Docs rule's existing behaviour is asserted alongside so the two rules are pinned to the same contract.

### Before

```
 ❯ packages/lexical-playground/__tests__/unit/DateTimeImport.test.ts (3 tests | 1 failed)
   × keeps the element content when the date cannot be parsed
     AssertionError: expected true to be false // Object.is equality
   ✓ still imports a parseable date
   ✓ matches how the Google Docs rule already handles an unparseable date

 Test Files  1 failed (1)
      Tests  1 failed | 2 passed (3)
```

### After

```
 Test Files  1 passed (1)
      Tests  3 passed (3)
```

Package suite is unchanged:

```
$ npx vitest run packages/lexical-playground
 Test Files  19 passed (19)
      Tests  277 passed (277)
```
@vercel

vercel Bot commented Aug 9, 2026

Copy link
Copy Markdown

@LeSingh1 is attempting to deploy a commit to the Meta Open Source Team on Vercel.

A member of the Team first needs to authorize it.

@meta-cla meta-cla 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 Aug 9, 2026
@mayrang

mayrang commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Ran the new test on main (1 failure as expected) and on this branch (3/3 pass). The isNaN guard mirrors GoogleDocsDateRule exactly. Three minor notes inline on the test file.

};

function importHtml(html: string): ImportResult {
using editor: LexicalEditor & Disposable = buildEditorFromExtensions(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The annotation and type import are redundant — buildEditorFromExtensions returns LexicalEditorWithDispose which already satisfies Disposable. The rest of the playground tests (e.g. CardNode.test.ts) use bare using editor = buildEditorFromExtensions(...).

import {PlaygroundImportExtension} from '../../src/nodes/PlaygroundImportExtension';
import {DateTimeExtension} from '../../src/plugins/DateTimeExtension';

const DateTimeImportTestExtension = /* @__PURE__ */ defineExtension({

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The /* @__PURE__ */ annotation on a test-only constant has no effect (tests aren't tree-shaken). The nearby playground tests drop it for defineExtension calls — this one stands out.

);

expect(result.hasDateTimeNode).toBe(true);
expect(result.serializes).toBe(true);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The other two cases assert result.text; this one doesn't. A regression that creates an empty DateTimeNode (dropping "May 29") would still pass. Consider adding expect(result.text).toBe('May 29').

@etrepum
etrepum added this pull request to the merge queue Aug 9, 2026
Merged via the queue into facebook:main with commit 9d90700 Aug 9, 2026
72 of 75 checks passed
@etrepum etrepum mentioned this pull request Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants