Skip to content

fix: resolve several sentry errors #1524

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Mar 17, 2025
Merged

fix: resolve several sentry errors #1524

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
36aaa5b
fix: if cannot find the next block, just stop looking
nperez0111 Mar 13, 2025
86f39c9
refactor: if no block, then no decorations
nperez0111 Mar 13, 2025
db4ce60
fix: if provided undefined, do not cause runtime errors
nperez0111 Mar 13, 2025
4c81741
don't touch type guards (for now)
YousefED Mar 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions packages/core/src/extensions/SuggestionMenu/getDefaultSlashMenuItems.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,15 @@ function setSelectionToNextContentEditableBlock<
I extends InlineContentSchema,
S extends StyleSchema
>(editor: BlockNoteEditor<BSchema, I, S>) {
let block = editor.getTextCursorPosition().block;
let block: Block<BSchema, I, S> | undefined =
editor.getTextCursorPosition().block;
let contentType = editor.schema.blockSchema[block.type].content;

while (contentType === "none") {
block = editor.getTextCursorPosition().nextBlock!;
block = editor.getTextCursorPosition().nextBlock;
if (block === undefined) {
return;
}
contentType = editor.schema.blockSchema[block.type].content as
| "inline"
| "table"
Expand Down
35 changes: 15 additions & 20 deletions packages/core/src/extensions/TableHandles/TableHandlesPlugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -656,32 +656,27 @@ export class TableHandlesProsemirrorPlugin<
}

const decorations: Decoration[] = [];

if (newIndex === this.view.state.draggingState.originalIndex) {
return DecorationSet.create(state.doc, decorations);
} else if (
this.view.state.draggingState.draggedCellOrientation === "row" &&
!canRowBeDraggedInto(
this.view.state.block,
this.view.state.draggingState.originalIndex,
newIndex
)
) {
return DecorationSet.create(state.doc, decorations);
} else if (
this.view.state.draggingState.draggedCellOrientation === "col" &&
!canColumnBeDraggedInto(
this.view.state.block,
this.view.state.draggingState.originalIndex,
newIndex
)
const { block, draggingState } = this.view.state;
const { originalIndex, draggedCellOrientation } = draggingState;

// Return empty decorations if:
// - Dragging to same position
// - No block exists
// - Row drag not allowed
// - Column drag not allowed
if (
newIndex === originalIndex ||
!block ||
(draggedCellOrientation === "row" &&
!canRowBeDraggedInto(block, originalIndex, newIndex)) ||
(draggedCellOrientation === "col" &&
!canColumnBeDraggedInto(block, originalIndex, newIndex))
) {
return DecorationSet.create(state.doc, decorations);
}

// Gets the table to show the drop cursor in.
const tableResolvedPos = state.doc.resolve(this.view.tablePos + 1);
const originalIndex = this.view.state.draggingState.originalIndex;

if (this.view.state.draggingState.draggedCellOrientation === "row") {
const cellsInRow = getCellsAtRowHandle(
Expand Down
20 changes: 14 additions & 6 deletions packages/core/src/schema/inlineContent/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,19 +126,27 @@ export type PartialInlineContent<
> = PartialInlineContentElement<I, T>[] | string;

export function isLinkInlineContent<T extends StyleSchema>(
content: InlineContent<any, T>
content: InlineContent<any, T> | undefined
Copy link
Collaborator

Choose a reason for hiding this comment

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

I want to be careful updating these types, what do these changes fix exactly / where was the error? shouldn't the typescript compiler break anyway if we had been passing undefined to the original functions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm I don't have the errors available right now (I should've linked them)

I typically go for a "be liberal in what you accept and strict in what you output" approach. Meaning that these guard functions should not choke on things like undefined or null.

But, I can also understand wanting to get to the root cause. I'll see if I can get what I resolved from Sentry

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was the sentry issue: https://blocknote-js.sentry.io/issues/32289262/?environment=vercel-production&query=is%3Aregressed&referrer=issue-stream&stream_index=2

I think you cannot assume that the index access into the array works, we should probably turn on noUncheckedIndexAccess on in our tsconfig: https://www.typescriptlang.org/tsconfig/#noUncheckedIndexedAccess

But, I stand by my previous statement that these functions are meant to assert that something is the correct type and therefore should also assert that something is there first before trying to assert that (i.e. check for undefined or null prior)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok. I usually take the opposite approach; https://martinfowler.com/ieeeSoftware/failFast.pdf - let's discuss when you're back :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd agree with that in general, but I see a type-guard as a parser: https://lexi-lambda.github.io/blog/2019/11/05/parse-don-t-validate/ 😉

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for that and for the issue! Let's discuss when you're back!

Copy link
Collaborator

Choose a reason for hiding this comment

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

fixed the issue at different level so we can discuss when you're back, just good to get the fixes in now

): content is Link<T> {
return content.type === "link";
return content !== undefined && content.type === "link";
}

export function isPartialLinkInlineContent<T extends StyleSchema>(
content: PartialInlineContentElement<any, T>
content: PartialInlineContentElement<any, T> | undefined
): content is PartialLink<T> {
return typeof content !== "string" && content.type === "link";
return (
content !== undefined &&
typeof content !== "string" &&
content.type === "link"
);
}

export function isStyledTextInlineContent<T extends StyleSchema>(
content: PartialInlineContentElement<any, T>
content: PartialInlineContentElement<any, T> | undefined
): content is StyledText<T> {
return typeof content !== "string" && content.type === "text";
return (
content !== undefined &&
typeof content !== "string" &&
content.type === "text"
);
}
Loading