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

fix(react-router): Fix config type import #15583

Merged
merged 2 commits into from
Mar 6, 2025

Conversation

namoscato
Copy link
Contributor

@namoscato namoscato commented Mar 5, 2025

@react-router/dev/dist/* is not a valid entry point. This results in sentryOnBuildEnd being typed as any as illustrated in this CI run.

The workaround is a type assertion in the interim, i.e. namoscato/tiny-truths@9c33bf1


Before submitting a pull request, please take a look at our
Contributing guidelines and verify:

  • If you've added code that should be tested, please add tests.
  • Ensure your code lints and the test suite passes (yarn lint) & (yarn test).

@namoscato namoscato marked this pull request as ready for review March 5, 2025 01:49
@lforst
Copy link
Member

lforst commented Mar 5, 2025

Thanks!

@lforst
Copy link
Member

lforst commented Mar 5, 2025

@chargome what's going on here? You remember why we have the dist here? Seems to fail TS without.

@chargome
Copy link
Member

chargome commented Mar 5, 2025

Damn yeah the config entrypoint breaks our ts build, which is why I used dist – din't realize until now that this types the hook any. Shuold we just vendor the type?

@lforst
Copy link
Member

lforst commented Mar 5, 2025

Yeah we can vendor it, but I would also advocate for contributing upstream to fix the typings

@chargome
Copy link
Member

chargome commented Mar 5, 2025

Their typings look alright, maybe this is something on our end

@namoscato
Copy link
Contributor Author

Ah yeah, sorry for not building this prior to opening the PR.

Here's the TypeScript error:

Cannot find module '@react-router/dev/config' or its corresponding type declarations.
  There are types at 'sentry-javascript/node_modules/@react-router/dev/dist/config.d.ts', but this result could not be resolved under your current 'moduleResolution' setting. Consider updating to 'node16', 'nodenext', or 'bundler'.

Currently, moduleResolution: 'node'. React Router requires Node.js >= v20, so think we'd want to update moduleResolution: 'node16', but that causes some other errors on glance.

@chargome chargome self-assigned this Mar 6, 2025
@chargome chargome merged commit 07d23bf into getsentry:develop Mar 6, 2025
135 checks passed
chargome added a commit that referenced this pull request Mar 6, 2025
This PR adds the external contributor to the CHANGELOG.md file, so that
they are credited for their contribution. See #15583

Co-authored-by: chargome <[email protected]>
@namoscato namoscato deleted the fix-react-router-import branch March 6, 2025 13:46
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.

3 participants