Fix and Type Tests (and some type errors I found along the way)

[nabbydude]

Description

This only changes types, test files, and configs.
Currently opening up anything JSX in a test directory highlights all the tags with errors. This fixes that and types all mocha test-runner files (which was necessary to solve the issue). Also consolidates most of the boilerplate between packages to top level files and JSX props have now been more definitively typed.

Also addresses some bugs I found along the way:

In slate/src

• Editor#setSelection was typed to allow Partial Ranges when it should be allowing any Partial Selection (which matters if Selection is CustomTyped to something beyond Range). This means it allows null now but it already handles that as a noop. Functionality is unchanged.

In slate/test

• fixed dozens of match predicates potentially returning undefined. (should matches be altered to accept this?)
• removed dozens of superfluous arguments and properties
• packages/slate/test/interfaces/Editor/levels/match.tsx was using non-existent property withMatch, I fixed it to be an actual match function but if it was still passing anyways this is probably a bad test. @12joan improved the test to be more meaningful. Thanks!
• packages/slate/test/interfaces/Node/elements/path.tsx and packages/slate/test/interfaces/Node/elements/range.tsx are using options of Node.elements that as far as I can tell never existed? The tests and the method they're testing have been unchanged since Next #3093. Disabled them until proper tests can be written. Also improved by @12joan. Thanks x2
• a few explicit type defs and as const casts.
• fixed many test files importing the jsx function from the wrong place (directly from slate-hyperscript) instead of the instance in test/index.ts

Context

I tried to avoid touching test files as much as possible to keep file changes to a minimum. EDIT: I had to alter imports for ~70 files so this is less true but ah well I still tried 😅

TSConfigs

I've added tsconfigs to each test directory so vscode and tsc can understand where to find types for checking purposes, they all inherit from a new file config/typescript/tsconfig.test.json, which has the noEmit flag set because mocha and jest can handle their own transpilation.

Types

support/types.ts contains the JSX definitions and CustomTypes extensions for nodes etc used in testing. Arbitrary properties used in testing have been added here, including every single letter as a flag, since I saw that used a lot. in the future I'd like to make this more robust, maybe using string template types as keys, something like so:

interface GenericAdditions {
  [k: `${string}_b`]: boolean | undefined
  [k: `${string}_f`]: true | undefined
}

to allow for arbitrary properties that are still typed.

The actual test/interfaces/CustomTypes suite had to be excluded from the tsconfig to avoid conflicts with the rest of the tests' types. It still runs in mocha just fine and it doesn't use any JSX so it seems to be happy in its own little corner.

Types in Libs

Enabled skipLibCheck in the base tsconfig to skip typechecking for dependencies. Should save some time and also it was giving me a strange error (claiming a package referenced by a package didnt have typedefs for a particular export when we clearly had the @types package for it and it definitely was exporting that type), I assume this was just some yarn pnp strangeness, it was a type-only import so I'm happy to write it off as not our problem. (@testing-library/jest-dom importing type ARIARole from aria-query, for anyone searching for the same issue in the future)

Implicit Any

Basically every one of these files has a function with an untyped parameter, which strict typescript doesn't like. To avoid changing nearly every single test file I just set the flag to allow this.

Implicit Any, JSX Edition

Turns out all JSX tags return the same type, no matter what the tag or its property is and no matter what the JSX function it gets compiled into is typed to return. This is frustratingly a limitation of typescript as a language.

This means the best we can type JSX values is as Node (and even that is assuming there's never a top-level selection tag). This would cause more errors as they try to get passed as more specific kinds like Text or Element but are too wide. So I have left them as any for now as well to avoid writing casts/typeguards in many more files.

Test Utils

JSX Tags with attributes void, inline, readOnly, and nonSelectable will create nodes that respond true to those respective functions. This was already the case but I made it attach during creation instead of being injected right before the test is run.

.spec.ts files

I added an exception to the fixtures function (which crawls for test files and run them), when it encounters a file ending in .spec.(extension) it will load it without wrapping it in a test, so that we can define our own suites and tests inside the files. I'm doing that now for tests I'm writing.

This scheme allows tests with the existing system to coexist with ones that want to group multiple tests together without having to add things to the index file.

Future

My focus for this PR was just getting types to work and not actually altering tests in a meaningful way, but someday™️ I'd love to improve upon them since it seems like (as is the case for many projects) they're pretty incomplete and even the ones we have aren't always the best. I'm gonna ramble about a wishlist but my thoughts are worth the same two cents as anyone else's:

• I think one file per test isn't really a great pattern, it makes for 1000+ v short files and a lot of code duplication. one file per suite/feature would mean that you can test multiple constraints of one feature/operation at the same time.
  • most tests use this { input, run, output } paradigm but (afaik) there's no way to make those values typesafe and what qualifies as the input and what just gets created inside run seems v unclear and arbitrary
• slate-dom currently has no tests at all.
• slate-react uses jest while the other packages use mocha, would love to consolidate this
• Personally I'm a fan of the expect(x).toEqual(y) paradigm but tbf it's really just sugar on top of assert (jest has it built in or chai provides it for mocha)
• Only tangentially related related to tests and related mostly in the way that its related to everything--which is to say it would make development and catching mistakes a little easier--but more jsdoc documentation would be nice, especially if it were automatically reflected into the gitbook docs.

Checks

• The new code matches the existing patterns and styles.
• The tests pass with yarn test.
  • Before: 1404 pass, 10 skipped
  • After: 1402 pass, 12 skipped (intentional. detailed above)
• The linter passes with yarn lint. (Fix errors with yarn fix.)
• The relevant examples still work. (Run examples with yarn start.)
• You've added a changeset if changing functionality. (Add one with yarn changeset add.)

[changeset-bot]

🦋 Changeset detected

Latest commit: 9f9d348

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

This PR includes changesets to release 1 package

Name	Type
slate	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

[12joan]

Awesome work!

[12joan]

I've created a PR against your PR branch to improve the tests you mentioned: nabbydude#1

[12joan]

fixed dozens of match predicates potentially returning undefined. (should matches be altered to accept this?)

Personally, I like the intentionality of always returning a boolean. It means you can't accidentally do something meaningless like match: node => { Text.isText(node) }.

For coercing values of type boolean | undefined, I tend to prefer !!value rather than value === true, but it's not a big issue.

I think one file per test isn't really a great pattern, it makes for 1000+ v short files and a lot of code duplication. one file per suite/feature would mean that you can test multiple constraints of one feature/operation at the same time.

Agreed. Needing to figure out where to put and how to name a new test file with the current pattern is a huge disincentive to writing tests.

slate-react uses jest while the other packages use mocha, would love to consolidate this

Definitely. I quite like Vitest, but switching everything to use Jest would also be nice.

[nabbydude]

ran in to some stumbling blocks while writing new tests for the main PR I'm working on, going to take a sec to disentangle the tweaks and push here, will convert to draft in meantime

[dylans]

@12joan does this still need changes?

[12joan]

It all looks good to me