import cycles in lexical playground

[aviv-skillset]

Remove import cycles from lexical-playground

import cycles are a dangerous anti-pattern and should be avoided. Honestly, I couldn't figure out a better way to implement this by myself and I think this should be discussed. Whether this issue should be resolved from the lexical framework API side ('lexical', '@lexical/react'...), or from the user side (in this case: lexical-playground is the user side)

cycle example: Image Node <-> Image Component

lexical/packages/lexical-playground/src/nodes/ImageComponent.tsx

Line 58 in c298349

import {$isImageNode} from './ImageNode';

lexical/packages/lexical-playground/src/nodes/ImageNode.tsx

Lines 251 to 255 in c298349

	export function $isImageNode(
	node: LexicalNode | null | undefined,
	): node is ImageNode {
	return node instanceof ImageNode;
	}

lexical/packages/lexical-playground/src/nodes/ImageComponent.tsx

Line 109 in c298349

export default function ImageComponent({

lexical/packages/lexical-playground/src/nodes/ImageNode.tsx

Lines 26 to 29 in c298349

	const ImageComponent = React.lazy(
	// @ts-ignore
	() => import('./ImageComponent'),
	);

This issue is also relevant for Table Node <-> Table Component

Thanks for reading. please leave a comment or hit a reaction button if you found this issue useful. if you found room for improvement, don't hesitate! please let me know

References:

https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/no-cycle.md
import-js/eslint-plugin-import#2265

[acywatson]

If you read the conversation in the github issue linked to that import rule, you'll see that dynamic imports are an acknowledged circumstance where "circular" dependencies can't have a practical impact at runtime. At the same time, the existence of these type guards on the same module as the node class is just a convention, so we could easily move them if we really wanted to, but I'm not seeing/hearing a compelling reason to do that yet.

[aviv-skillset]

First of all, thank you for the quick response. This is much appreciated.

the type guard is the part that I would not modify, since to create this type guard in another file you will be required to import the node so you can use "instance of".
I think that in this case, wrapping the image component with a context provider can help drill down this function without any cycles.

about the cycles, I didn't conclude their conversation as you did. the TL;DR for me was "cycles are bad" yet not always harmful.
the facts are that the code works, and I can't argue with that, yet I believe that a playground should suggest the best practices, in this case regarding how a lexical app should look like.

[thegreatercurve]

I think this is generally fine. We've not seen any increases or duplication in bundle size which circular dependencies can create. I'm not necessarily sure we need to be injecting code down via Context, a simpler solution would be just to put it in another module.