Skip to content

fix: repo's typescript and ESlint configuration and errors - #796

Open
hejsztynx wants to merge 9 commits into
mainfrom
@ksienkiewicz/fix-repo-ts-lint
Open

hejsztynx wants to merge 9 commits into
mainfrom
@ksienkiewicz/fix-repo-ts-lint

Conversation

@hejsztynx

@hejsztynx hejsztynx commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Summary

I've done some changes to TypeScript and ESlint related changes to fix already existing errors in the codebase. These errors were not caught by the project's scripts like yarn lint or yarn typecheck, but were visible in the IDE. This is probably because of the differences how npm and IDE runs TS. I've verified that these errors were present in two different IDEs - VSCode and Zed on different machines, so it's a shared issue and worth fixing even if they don't persist into CI.

  1. Pinned root's tsconfig.json types resolution paths, as we had react installed in both root and apps/example node_modules, and TS would get confused resulting in errors, e.g. in src/native/EnrichedTextInput.tsx.
// Argument of type 'null' is not assignable to parameter of type 'never'.
focus: () => {
     Commands.focus(nullthrows(nativeRef.current));
},

A similar issue was already mentioned in apps/example/tsconfig.json:

      // Pin a single `@types/react` copy so the library source typechecked here
      // doesn't mix react types with the example's copy (which makes codegen
      // command ref types collapse to `never`).
      "react": ["../../node_modules/@types/react"],
      "react/*": ["../../node_modules/@types/react/*"]
  1. With the RN 0.87 bump, we also bumped root TypeScript to 6.x, and in example-web we still use TS 5.x. I believe it's better to have the example-web use the root TS, just like the apps/example already does, and have a single TS across the repo, whenever possible

  2. example-web had its own separate ESLint and the its code wasn't linted to those rules. There were two options: tweak the example-web code so it respects the configured standalone ESLint, or remove that ESLint and let the root ESLint handle the example-web. I opt for the latter one, as we keep linting rules consistent across the repo, we don't have to maintain another ESLint configuration, it doesn't require modifying the example app's code, and we've been using the root ESLint in example-web via yarn lint anyway

  3. In .playwright, the TS wasn't setup correctly and we got errors e.g. in images.spec.ts, we got:

Cannot find name 'Buffer'. Do you need to install type definitions for node? Try `npm i --save-dev @types/node` and then add 'node' to the types field in your tsconfig.

I added the @types/nodes package and wired up .playwright with TS

  1. Linted the code where it wasn't.

  2. Removed deprecated React.ElementRef in favor of React.ComponentRef in src/spec/EnrichedTextInputNativeComponent.ts

Test Plan

What I've verified:

  • yarn test
  • yarn test:e2e:web
  • yarn test:e2e:mobile - the tests build and run, but I haven't run them all through
  • yarn ios
  • yarn android
  • yarn lint
  • yarn typecheck

You can also run these scripts - everything should pass

Checklist

  • E2E tests are passing
  • Required E2E tests have been added (if applicable)

Copilot AI lite review requested due to automatic review settings September 11, 2026 10:42
@hejsztynx
hejsztynx marked this pull request as draft September 11, 2026 10:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Critical configuration and dependency issues remain unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR centralizes TypeScript and ESLint configuration, adds Playwright typechecking, updates typings, and applies lint fixes.

Changes:

  • Pins React type resolution and aligns example-web tooling with the root configuration.
  • Adds Playwright TypeScript configuration and Node typings.
  • Replaces deprecated React ref types and updates dependency resolutions.
File summaries
File Summary
yarn.lock Dependency updates; critical Tiptap peer-version mismatch remains (1 vote).
tsconfig.json Pins React types and updates exclusions.
src/web/utils/pasteImages.ts Formatting update.
src/web/nativeMappers/returnKeyTypeToEnterKeyHint.ts Formatting update.
src/spec/EnrichedTextInputNativeComponent.ts Replaces deprecated React.ElementRef.
package.json Adds Playwright typechecking and Node typings; critical .playwright may still be included by typecheck:lib (1 vote).
apps/example/src/components/ToolbarButton.tsx Formatting update.
apps/example-web/tsconfig.app.json Pins React typings; moderate Turbo inputs do not include this configuration (1 vote).
apps/example-web/package.json Removes local tooling dependencies; critical scripts may no longer resolve required binaries (1 vote).
apps/example-web/eslint.config.mjs Removes duplicate ESLint configuration.
.playwright/tsconfig.json Adds Playwright TypeScript configuration.
.playwright/tests/headingBoldFromStyle.spec.ts Moderate empty heading captures may be treated as missing (2 votes).
.playwright/tests/codeblockInlineStyles.spec.ts Updates regex capture handling.
Review details

Suppressed comments (1)

apps/example-web/tsconfig.app.json:12

  • This PR changes tsconfig.app.json, but the build:web Turbo inputs only include tsconfig.json (turbo.json:9). As a result, a cached web build can be reused without hashing these new path mappings, so CI/local Turbo runs may not rebuild after this configuration changes. Add tsconfig.app.json (or a matching tsconfig*.json input) to the task inputs.
      "react": ["../../node_modules/@types/react"],
      "react/*": ["../../node_modules/@types/react/*"],
      "react-dom": ["../../node_modules/@types/react-dom"],
      "react-dom/*": ["../../node_modules/@types/react-dom/*"],
    },
  • Files reviewed: 11/13 changed files
  • Comments generated: 3
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread apps/example-web/package.json
Comment thread package.json
Comment thread .playwright/tests/headingBoldFromStyle.spec.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Address the web build typechecking gap and exclude .playwright from the root TypeScript project.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 12/14 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread apps/example-web/package.json
"build:web": "tsc -b && vite build",
"preview": "vite preview",
"lint": "eslint . --ext ts,tsx"
"build:web": "vite build",

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I believe we can safely remove that tsc call. It was there, because we had a standalone TS, but now we use the root one and we use the root yarn typecheck

@hejsztynx
hejsztynx marked this pull request as ready for review September 11, 2026 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants