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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix feature detection of React.startTransition #10569

Merged
merged 1 commit into from Jun 8, 2023
Merged

Fix feature detection of React.startTransition #10569

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/silly-eagles-divide.md
View file
Open in desktop
@@ -0,0 +1,6 @@
---
"react-router": patch
"react-router-dom": patch
---

Adjust feature detection of `React.startTransition` to fix webpack + react 17 compilation error
17 changes: 11 additions & 6 deletions packages/react-router-dom/index.tsx
View file
Open in desktop
Expand Up @@ -300,6 +300,11 @@ export interface BrowserRouterProps {
window?: Window;
}

// Webpack + React 17 fails to compile on the usage of `React.startTransition` or
// `React["startTransition"]` even if it's behind a feature detection of
// `"startTransition" in React`. Moving this to a constant avoids the issue :/
Comment on lines +303 to +305
Copy link

Choose a reason for hiding this comment

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

I'd assume it's tied to the import * as React from "react" import, the more typical usage is import React from "react".

Any chance of shipping this as is to un-break consumers btw? It's having pretty broad impact

Copy link
Contributor Author

Choose a reason for hiding this comment

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

import React from "react" is deprecated as of React 17

https://twitter.com/dan_abramov/status/1308739731551858689

Copy link

Choose a reason for hiding this comment

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

Good to know! 馃憤 Either way it's likely the reason why you saw this issue here but not in other places.

const START_TRANSITION = "startTransition";

/**
* A `<Router>` for use in web browsers. Provides the cleanest URLs.
*/
Expand All @@ -320,8 +325,8 @@ export function BrowserRouter({
});
let setState = React.useCallback(
(newState: { action: NavigationType; location: Location }) => {
"startTransition" in React
? React.startTransition(() => setStateImpl(newState))
START_TRANSITION in React
? React[START_TRANSITION](() => setStateImpl(newState))
: setStateImpl(newState);
},
[setStateImpl]
Expand Down Expand Up @@ -363,8 +368,8 @@ export function HashRouter({ basename, children, window }: HashRouterProps) {
});
let setState = React.useCallback(
(newState: { action: NavigationType; location: Location }) => {
"startTransition" in React
? React.startTransition(() => setStateImpl(newState))
START_TRANSITION in React
? React[START_TRANSITION](() => setStateImpl(newState))
: setStateImpl(newState);
},
[setStateImpl]
Expand Down Expand Up @@ -402,8 +407,8 @@ function HistoryRouter({ basename, children, history }: HistoryRouterProps) {
});
let setState = React.useCallback(
(newState: { action: NavigationType; location: Location }) => {
"startTransition" in React
? React.startTransition(() => setStateImpl(newState))
START_TRANSITION in React
? React[START_TRANSITION](() => setStateImpl(newState))
: setStateImpl(newState);
},
[setStateImpl]
Expand Down
13 changes: 9 additions & 4 deletions packages/react-router/lib/components.tsx
View file
Open in desktop
Expand Up @@ -54,6 +54,11 @@ export interface RouterProviderProps {
router: RemixRouter;
}

// Webpack + React 17 fails to compile on the usage of `React.startTransition` or
// `React["startTransition"]` even if it's behind a feature detection of
// `"startTransition" in React`. Moving this to a constant avoids the issue :/
const START_TRANSITION = "startTransition";

/**
* Given a Remix Router instance, render the appropriate UI
*/
Expand All @@ -66,8 +71,8 @@ export function RouterProvider({
let [state, setStateImpl] = React.useState(router.state);
let setState = React.useCallback(
(newState: RouterState) => {
"startTransition" in React
? React.startTransition(() => setStateImpl(newState))
START_TRANSITION in React
? React[START_TRANSITION](() => setStateImpl(newState))
: setStateImpl(newState);
},
[setStateImpl]
Expand Down Expand Up @@ -178,8 +183,8 @@ export function MemoryRouter({
});
let setState = React.useCallback(
(newState: { action: NavigationType; location: Location }) => {
"startTransition" in React
? React.startTransition(() => setStateImpl(newState))
START_TRANSITION in React
? React[START_TRANSITION](() => setStateImpl(newState))
: setStateImpl(newState);
},
[setStateImpl]
Expand Down