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.

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

Jump to bottom

Update prosemirror-*, remove @types/prosemirror-* #1359

Merged
merged 11 commits into from
Nov 15, 2022
Merged

Update prosemirror-*, remove @types/prosemirror-* #1359

merged 11 commits into from
Nov 15, 2022

Conversation

kachkaev
Copy link
Contributor

@kachkaev kachkaev commented Nov 8, 2022 •
edited

🌟 What is the purpose of this PR?

When updating devDependencies in #1353, I noticed that @types/prosemirror-* packages got deprecated (e.g. @types/prosemirror-view). It turned out that the typings were moved into ProseMirror packages themselves.

This PR removes @types/prosemirror-* and updates all prosemirror-* packages to the latest available versions. They are semver-minor, so should be backwards-compatible with the versions we've had. The main change is the removal of generic Schema argument from all types.

🔗 Related links

📜 Does this require a change to the docs?

  • No

⚠️ Known issues

See PR comment below

🐾 Next steps

Configure autoupdates for prosemirror deps

🛡 What tests cover this?

  • CI

❓ How to test this?

  1. Check CI (Playwright tests)

@github-actions github-actions bot added area/apps > hash* Affects HASH (a `hash-*` app) area/infra Relates to version control, CI, CD or IaC (area) area/apps > hash-api Affects the HASH API (app) area/deps Relates to third-party dependencies (area) frontend labels Nov 8, 2022
kachkaev
kachkaev commented Nov 8, 2022
View reviewed changes
package.json
@@ -50,7 +50,7 @@
"lint:prettier": "prettier --check --ignore-unknown .",
"lint:tsc": "turbo run --continue lint:tsc",
"lint:yarn-deduplicate": "yarn-deduplicate --fail --list --strategy=fewer",
"postinstall": "patch-package && husky install && yarn codegen",
"postinstall": "patch-package --error-on-warn && husky install && yarn codegen",
Copy link
Contributor Author

@kachkaev kachkaev Nov 8, 2022 •
edited

Choose a reason for hiding this comment

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

Bonus, see justification in: blockprotocol/blockprotocol#736

@github-actions github-actions bot added the area/blocks Relates to first-party blocks (area) label Nov 8, 2022
kachkaev
kachkaev commented Nov 8, 2022
View reviewed changes
blocks/paragraph/package.json Outdated
@@ -30,8 +30,7 @@
"mock-block-dock": "0.0.38",
"react": "^18.2.0",
"react-dom": "^18.2.0",
"typescript": "4.8.4",
"typescript-json-schema": "0.54.0"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bonus: follow-up from #1353 (comment)

@kachkaev kachkaev mentioned this pull request Nov 8, 2022
Merged
1 task
@kachkaev kachkaev changed the title Update Prosemirror dependencies Update prosemirror-*, remove @types/prosemirror-* Nov 8, 2022
@kachkaev
Copy link
Contributor Author

kachkaev commented Nov 8, 2022 •
edited

Getting this from lint:tsc:

@hashintel/hash-shared:lint:tsc: src/prosemirror.ts(235,12): error TS2339: Property 'groups' does not exist on type 'NodeType'.
@hashintel/hash-shared:lint:tsc: src/prosemirror.ts(314,17): error TS2339: Property 'groups' does not exist on type 'NodeType'.
@hashintel/hash-shared:lint:tsc: src/prosemirror.ts(315,19): error TS2339: Property 'groups' does not exist on type 'NodeType'.
@hashintel/hash-shared:lint:tsc: src/prosemirror.ts(330,17): error TS2540: Cannot assign to 'schema' because it is a read-only property.
@hashintel/hash-shared:lint:tsc: src/prosemirror.ts(331,11): error TS2542: Index signature in type '{ readonly [x: string]: NodeType; } & { readonly [key: string]: NodeType; }' only permits reading.
@hashintel/hash-shared:lint:tsc: src/prosemirror.ts(346,17): error TS2540: Cannot assign to 'schema' because it is a read-only property.
@hashintel/hash-shared:lint:tsc: src/prosemirror.ts(347,11): error TS2542: Index signature in type '{ readonly [x: string]: MarkType; } & { readonly [key: string]: MarkType; }' only permits reading

The rest looks clean. @nathggns thoughts? (feel free to push if you want)

UPD All CI checks pass now.

kachkaev
kachkaev commented Nov 8, 2022
View reviewed changes
libs/javascript/@local/eslint-config/legacy-base-eslintrc-to-refactor.cjs
@@ -28,71 +28,22 @@ module.exports = {
"error",
{
selector:
"TSTypeReference[typeName.name=/^(DirectEditorProps|EditorProps|EditorView|EditorState|NodeView|ProsemirrorNode|Command|Transaction|Plugin|PluginKey)$/]:not([typeParameters])",
"TSTypeReference[typeName.name=/^(Plugin|PluginKey)$/]:not([typeParameters])",
Copy link
Contributor Author

@kachkaev kachkaev Nov 8, 2022 •
edited

Choose a reason for hiding this comment

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

Plugin and PluginKey are still generic (one arg instead of two)

@kachkaev kachkaev marked this pull request as ready for review November 8, 2022 16:02
@github-actions github-actions bot removed the area/blocks Relates to first-party blocks (area) label Nov 14, 2022
kachkaev
kachkaev commented Nov 15, 2022
View reviewed changes
packages/hash/shared/src/prosemirror.ts
Comment on lines +330 to 333
// @ts-expect-error -- NodeType#schema is readonly in prosemirror-model
value.schema = schema;
// @ts-expect-error -- NodeType#nodes is readonly in prosemirror-model
this.nodes[key] = value;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nathggns do you see any possible issues with these @ts-expect-error instances? There are two more just below.

@nathggns
Copy link
Contributor

Reviewing this now

@nathggns nathggns merged commit 6ba5635 into main Nov 15, 2022
@nathggns nathggns deleted the ak/prosemirror-deps branch November 15, 2022 17:38
This was referenced Nov 15, 2022
Merged
Merged
@vilkinsons vilkinsons added type/eng > frontend Owned by the @frontend team and removed area: frontend labels Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nathggns nathggns nathggns approved these changes

Assignees
No one assigned
Labels
area/apps > hash* Affects HASH (a `hash-*` app) area/apps > hash-api Affects the HASH API (app) area/deps Relates to third-party dependencies (area) area/infra Relates to version control, CI, CD or IaC (area) type/eng > frontend Owned by the @frontend team
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@kachkaev @nathggns @vilkinsons