-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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(remix-server-runtime): don't use React types #5713
fix(remix-server-runtime): don't use React types #5713
Conversation
🦋 Changeset detectedLatest commit: cd1b030 The changes in this PR will be included in the next version bump. This PR includes changesets to release 18 packages
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 |
5837f1f
to
7f5ac90
Compare
0999b33
to
07fb98f
Compare
e95c1de
to
90061ef
Compare
90061ef
to
a118640
Compare
1376cfc
to
e9b7236
Compare
e9b7236
to
4d5ecb5
Compare
7c07a00
to
f4b7b12
Compare
f4b7b12
to
f9a0363
Compare
aff997f
to
05144a3
Compare
Talked this over with @jacob-ebey and @pcattori and we decided that the loosening of the types in server-runtime is really just a bug fix since that should never have been exporting react-flavored types. We've included deprecation warnings pointing to the corresponding types in |
b9c3091
to
c88fabb
Compare
Do I understand correctly based on the labels that this won't be fixed until v2? When will that be? |
c88fabb
to
1db1f26
Compare
1db1f26
to
cd1b030
Compare
@lpsinger Nah sorry for the confusion - we're going to get this merged for the next release |
🤖 Hello there, We just published version Thanks! |
🤖 Hello there, We just published version Thanks! |
Hm, we must have a bug in our bot logic - this is not included in https://github.com/remix-run/remix/compare/remix@1.17.0...remix@1.17.1-pre.1 |
yeah that’s odd… it gets the commits via |
🤖 Hello there, We just published version Thanks! |
🤖 Hello there, We just published version Thanks! |
🤖 Hello there, We just published version Thanks! |
🤖 Hello there, We just published version Thanks! |
Follow-up of #4801
Not the most ideal thing to do imo, but it fixes #5615 🤷♂️
As I mentioned in #4801 (comment), I added
@types/react
todependencies
because we're usingComponentType
type inpackages/remix-server-runtime/routeModules.ts
remix/packages/remix-server-runtime/routeModules.ts
Line 40 in 7935312
remix/packages/remix-server-runtime/routeModules.ts
Line 45 in 7935312
remix/packages/remix-server-runtime/routeModules.ts
Line 218 in 7935312
We should probably move that to
@remix-run/react
and make@remix-run/server-runtime
more renderer-agnostic in that place.@brophdawg11 thought that would be the case when moving to RR 6.4 (see #4801 (comment)), but apparently that's not the case 😢
Closes #5615
Closes #6275