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

Inline startTransitionImpl to avoid .d.ts issues with skipLibCheck:false #10622

Merged
merged 2 commits into from
Jun 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/tsc-skiplibcheck-react17.md
@@ -0,0 +1,6 @@
---
"react-router": patch
"react-router-dom": patch
---

Fix `tsc --skipLibCheck:false` issues on React 17
25 changes: 24 additions & 1 deletion packages/react-router-dom/index.tsx
Expand Up @@ -27,7 +27,6 @@ import {
UNSAFE_NavigationContext as NavigationContext,
UNSAFE_RouteContext as RouteContext,
UNSAFE_mapRouteProperties as mapRouteProperties,
UNSAFE_startTransitionImpl as startTransitionImpl,
UNSAFE_useRouteId as useRouteId,
} from "react-router";
import type {
Expand Down Expand Up @@ -297,6 +296,30 @@ function deserializeErrors(
//#region Components
////////////////////////////////////////////////////////////////////////////////

/**
Webpack + React 17 fails to compile on any of the following because webpack
complains that `startTransition` doesn't exist in `React`:
* import { startTransition } from "react"
* import * as React from from "react";
"startTransition" in React ? React.startTransition(() => setState()) : setState()
* import * as React from from "react";
"startTransition" in React ? React["startTransition"](() => setState()) : setState()

Moving it to a constant such as the following solves the Webpack/React 17 issue:
* import * as React from from "react";
const START_TRANSITION = "startTransition";
START_TRANSITION in React ? React[START_TRANSITION](() => setState()) : setState()

However, that introduces webpack/terser minification issues in production builds
in React 18 where minification/obfuscation ends up removing the call of
React.startTransition entirely from the first half of the ternary. Grabbing
this exported reference once up front resolves that issue.

See https://github.com/remix-run/react-router/issues/10579
*/
const START_TRANSITION = "startTransition";
const startTransitionImpl = React[START_TRANSITION];

export interface BrowserRouterProps {
basename?: string;
children?: React.ReactNode;
Expand Down
2 changes: 0 additions & 2 deletions packages/react-router/index.ts
Expand Up @@ -43,7 +43,6 @@ import {
UNSAFE_warning as warning,
} from "@remix-run/router";

import startTransitionImpl from "./lib/polyfills/start-transition";
import type {
AwaitProps,
MemoryRouterProps,
Expand Down Expand Up @@ -304,5 +303,4 @@ export {
mapRouteProperties as UNSAFE_mapRouteProperties,
useRouteId as UNSAFE_useRouteId,
useRoutesImpl as UNSAFE_useRoutesImpl,
startTransitionImpl as UNSAFE_startTransitionImpl,
};
25 changes: 24 additions & 1 deletion packages/react-router/lib/components.tsx
Expand Up @@ -22,7 +22,6 @@ import {
UNSAFE_getPathContributingMatches as getPathContributingMatches,
} from "@remix-run/router";

import startTransitionImpl from "./polyfills/start-transition";
import type {
DataRouteObject,
IndexRouteObject,
Expand Down Expand Up @@ -60,6 +59,30 @@ export interface RouterProviderProps {
future?: FutureConfig;
}

/**
Webpack + React 17 fails to compile on any of the following because webpack
complains that `startTransition` doesn't exist in `React`:
* import { startTransition } from "react"
* import * as React from from "react";
"startTransition" in React ? React.startTransition(() => setState()) : setState()
* import * as React from from "react";
"startTransition" in React ? React["startTransition"](() => setState()) : setState()

Moving it to a constant such as the following solves the Webpack/React 17 issue:
* import * as React from from "react";
const START_TRANSITION = "startTransition";
START_TRANSITION in React ? React[START_TRANSITION](() => setState()) : setState()

However, that introduces webpack/terser minification issues in production builds
in React 18 where minification/obfuscation ends up removing the call of
React.startTransition entirely from the first half of the ternary. Grabbing
this exported reference once up front resolves that issue.

See https://github.com/remix-run/react-router/issues/10579
*/
const START_TRANSITION = "startTransition";
const startTransitionImpl = React[START_TRANSITION];

/**
* Given a Remix Router instance, render the appropriate UI
*/
Expand Down
28 changes: 0 additions & 28 deletions packages/react-router/lib/polyfills/start-transition.ts

This file was deleted.