-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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] Feature: Add version identifier to LexicalEditor constructor #6488
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
size-limit report 📦
|
I think these are flaky collab test failures |
@@ -84,7 +84,7 @@ handle external UI state and UI features relating to specific types of node. | |||
If any existing nodes are in the DOM, and skipInitialization is not true, the listener | |||
will be called immediately with an updateTag of 'registerMutationListener' where all | |||
nodes have the 'created' NodeMutation. This can be controlled with the skipInitialization option | |||
(default is currently true for backwards compatibility in 0.16.x but will change to false in 0.17.0). | |||
(default is currently true for backwards compatibility in 0.17.x but will change to false in 0.18.0). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed the deprecation policy here since 0.17.0 was the first release with the changes to registerMutationListener
(#6357)
@@ -52,6 +52,10 @@ export default defineConfig(({command}) => { | |||
from: /__DEV__/g, | |||
to: 'true', | |||
}, | |||
{ | |||
from: 'process.env.LEXICAL_VERSION', | |||
to: JSON.stringify(`${process.env.npm_package_version}+git`), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The playground is never built with an npm release of lexical since it directly includes all of the source
…ly in prod builds
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
return activeEditor; | ||
} | ||
|
||
function collectBuildInformation(): string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@etrepum nitpicking but such detailed error is something I'd consider gating under the __DEV__
build or developer extension. I don't think it's very common to have multiple editors on the same page and it's less common to make this mistake, and there is quite some logic in here 679 bytes minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the amount of bug reports for this one issue I was motivated to include it, I think a lot of people end up with the prod build for various reasons. You could put it behind a flag so it doesn't get included in www? Once dev tools gets a release with this functionality we could stub the implementation out with a string that says to install the dev tools to diagnose. We are still talking about <1kb before compression.
Description
New
LexicalEditor.version
static to help users diagnose installation issues.Also refactors other
__lexicalEditor
code to avoid conflicts with multiple lexical builds on the page (RE #6481)Inspecting the lexical editor's version can be done as follows (example assumes exactly one editor on the page):
It does not have support for coming up with special build identifiers for the versions of lexical we build in CI and for the website that do not correspond to npm versions.
Closes #6145
Closes #5996
Test plan
There's an integration test that checks to make sure the version is set and accessible from the editor instance
Before
After
Using https://lexical-playground-git-fork-etrepum-version-a6c103-fbopensource.vercel.app/esm/