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

[lexical] Add tests for HTMLConfig #5507

Merged
merged 7 commits into from
Aug 26, 2024

Conversation

2wheeh
Copy link
Contributor

@2wheeh 2wheeh commented Jan 18, 2024

EDIT:
This adds tests for HTMLConfig

Copy link

vercel bot commented Jan 18, 2024

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 Aug 25, 2024 1:55pm
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 25, 2024 1:55pm

Copy link

github-actions bot commented May 22, 2024

size-limit report 📦

Path Size
lexical - cjs 29.38 KB (0%)
lexical - esm 29.24 KB (0%)
@lexical/rich-text - cjs 37.89 KB (0%)
@lexical/rich-text - esm 31.07 KB (0%)
@lexical/plain-text - cjs 36.49 KB (0%)
@lexical/plain-text - esm 28.41 KB (0%)
@lexical/react - cjs 39.61 KB (0%)
@lexical/react - esm 32.52 KB (0%)

Copy link
Collaborator

@etrepum etrepum left a comment

Choose a reason for hiding this comment

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

I like that this adds tests but it does let you write incorrect code that passes the type checker because really what we would want to say is that the key and value of the map are related (something like forall T extends LexicalNode. [K: Klass<T>, V: (editor: LexicalEditor, target: T) => DOMExportOutput] but I don't think it would be possible to express something like that directly with Map's type

@2wheeh 2wheeh changed the title Make target node type to generic on HTMLConfig [lexical] Add tests for HTMLconfig Aug 25, 2024
@2wheeh 2wheeh changed the title [lexical] Add tests for HTMLconfig [lexical] Add tests for HTMLConfig Aug 25, 2024
@2wheeh
Copy link
Contributor Author

2wheeh commented Aug 25, 2024

I don't think it would be possible to express something like that directly with Map's type

@etrepum You are right. I reverted that and changed this PR to focus on adding unit tests for HTMLConfig.

Copy link
Collaborator

@etrepum etrepum left a comment

Choose a reason for hiding this comment

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

Thank you!

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. extended-tests Run extended e2e tests on a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants