[lexical-table] Chore: Lower table handler command priority#7933
[lexical-table] Chore: Lower table handler command priority#7933etrepum merged 2 commits intofacebook:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
💡 Enable Vercel Agent with $100 free credit for automated AI reviews |
etrepum
left a comment
There was a problem hiding this comment.
I think generally this is fine, but might be worth noting that these priorities have changed in as a potentially Breaking Change.
IMO the priority system is not good especially with the commands as being as coarse as they are. Decomposing them into smaller commands will help, but eventually we're going to want to have a way to register command listeners in a way that go before, after, or around some other listener (around is really the general case since you can implement before or after with that primitive).
You could mostly get by without having any specific targeting if you could register listeners around all existing listeners, but the current behavior is that new listeners are added at a lower priority which is the least useful order. You almost always want your new listener that depends on existing listeners to run first so you can choose to run the existing listeners or not, but the only way to do that today is to register something at a higher priority and you run out of those quickly.
Description
LexicalTableSelectionHelpersintercepts a number of commands with critical priority. This makes it difficult for custom plugins to intercept those same commands if TablePlugin is present (the plugin would need to be registered after TablePlugin).I checked git history and could not see any reason for them to be critical priority (other than the fact that they are important for table operations). They were created before priorities were constant values.
Generally, I think any command should be able to be intercepted, so core handlers using
COMMAND_PRIORITY_CRITICALis unexpected to me.If
CRITICALis the correct priority here, I can add a section to the docs on plugin ordering.Use case
My specific use case is intercepting
CUT_COMMANDin a plugin that prevents edits to content under certain situations. If table plugin is registered after, it's not possible for my plugin to prevent deletes via CUT commands.Test plan
Existing tests pass, QA table operations (tab, cut, delete etc)
Before
Custom plugins cannot intercept commands registered by TablePlugin
After
Custom plugins can intercept commands by using
COMMAND_PRIORITY_CRITICAL