Skip to content
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

add guard checks for value and editor in <Slate /> #3326

Merged

Conversation

rockettomatooo
Copy link
Contributor

@rockettomatooo rockettomatooo commented Dec 15, 2019

Is this adding or improving a feature or fixing a bug?

Bug

What's the new behavior?

An error message is displayed when the user passes faulty values to <Slate />:
image
(I passed null as value)

How does this change work?

value and editor props passed to <Slate /> are checked with Node.isNodeList() and Editor.isEditor().

Have you checked that...?

  • The new code matches the existing patterns and styles.
  • The tests pass with yarn test.
  • The linter passes with yarn lint. (Fix errors with yarn fix.)
  • The relevant examples still work. (Run examples with yarn start.)

Does this fix any issues or need any specific reviewers?

--

@rockettomatooo
Copy link
Contributor Author

Unfortunately, there are no tests setup for slate-react. I tested this with an example (See the screnshot above).

Also the linter doesn't like my console.error(). I dunno if it's maybe better to throw an error instead (for a stacktrace maybe) but that would also kill the whole editor.

@ianstormtaylor
Copy link
Owner

Hey @rockettomatooo can you use the tiny-invariant package for this? That's what we've used in the past.

Also I think those lines should be moved inside the useMemo callback, just to make them slightly less frequent (although it's still fairly often).

@rockettomatooo
Copy link
Contributor Author

Hi @ianstormtaylor I now added the tiny-invariant package as a dependency to slate-react and moved the guard checks into the useMemo callback.
Also I merged my branch with the current master as there was a conflict with my first commit.

@cameracker
Copy link
Collaborator

(FYI I resolved the merge conflict in package.json)

@changeset-bot
Copy link

changeset-bot bot commented Mar 31, 2021

🦋 Changeset detected

Latest commit: 104e1f3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
slate-react Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@ianstormtaylor
Copy link
Owner

Thanks @rockettomatooo! Sorry for the delay.

@ianstormtaylor ianstormtaylor merged commit d5b2d7f into ianstormtaylor:master Mar 31, 2021
This was referenced Mar 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants