Skip to content

Commit

Permalink
Make proper use of view.someProp
Browse files Browse the repository at this point in the history
There were a few places where we incorrectly used view.someProp to
retrieve a prop value, and then did something with the value. This will
_always_ result in the first value being returned, even in cases where
multiple plugins provide a value for the prop, and only some of them
are relevant at the callsite.

Instead, we now pass a function to view.someProp that only returns a
truthy value when it has found a prop provider that provides the
relevant value, so that we can fallback through the entire list of
plugins.
  • Loading branch information
smoores-dev committed May 6, 2024
1 parent c90da83 commit d6ea3ad
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 257 deletions.
28 changes: 14 additions & 14 deletions docs/assets/index-7c1043a3.js → docs/assets/index-474d9858.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion docs/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<link rel="icon" type="image/svg+xml" href="/vite.svg" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>React-ProseMirror Demo</title>
<script type="module" crossorigin src="/react-prosemirror/assets/index-7c1043a3.js"></script>
<script type="module" crossorigin src="/react-prosemirror/assets/index-474d9858.js"></script>
<link rel="stylesheet" href="/react-prosemirror/assets/index-523693cd.css">
</head>
<body>
Expand Down
17 changes: 13 additions & 4 deletions src/components/NodeView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,10 @@ export function NodeView({

// TODO: Would be great to pull all of the custom node view stuff into
// a hook
const customNodeView = view?.someProp("nodeViews")?.[node.type.name];
const customNodeView = view?.someProp(
"nodeViews",
(nodeViews) => nodeViews?.[node.type.name]
);

useLayoutEffect(() => {
if (!customNodeViewRef.current || !customNodeViewRootRef.current) return;
Expand All @@ -92,15 +95,18 @@ export function NodeView({

destroy?.call(customNodeViewRef.current);

if (!customNodeView || !customNodeViewRootRef.current) return;
if (!customNodeViewRootRef.current) return;

initialNode.current = node;
initialOuterDeco.current = outerDeco;
initialInnerDeco.current = innerDeco;

customNodeViewRef.current = customNodeView(
initialNode.current,
view,
// customNodeView will only be set if view is set, and we can only reach
// this line if customNodeView is set
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
view!,
() => posRef.current,
initialOuterDeco.current,
initialInnerDeco.current
Expand Down Expand Up @@ -142,7 +148,10 @@ export function NodeView({
if (!customNodeViewRef.current) {
customNodeViewRef.current = customNodeView(
initialNode.current,
view,
// customNodeView will only be set if view is set, and we can only reach
// this line if customNodeView is set
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
view!,
() => posRef.current,
initialOuterDeco.current,
initialInnerDeco.current
Expand Down
237 changes: 0 additions & 237 deletions src/hooks/useBeforeInput.ts

This file was deleted.

2 changes: 1 addition & 1 deletion src/plugins/beforeInputPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ function insertText(
const from = options.from ?? view.state.selection.from;
const to = options.to ?? view.state.selection.to;

if (view.someProp("handleTextInput")?.(view, from, to, eventData)) {
if (view.someProp("handleTextInput", (f) => f(view, from, to, eventData))) {
return true;
}

Expand Down

0 comments on commit d6ea3ad

Please sign in to comment.