Skip to content

Commit

Permalink
Merge branch 'dev' into pedro/stop-dev-when-esbuild-crashes
Browse files Browse the repository at this point in the history
  • Loading branch information
pcattori authored Aug 14, 2023
2 parents 18db3e5 + 7a9ed0b commit 7a88a00
Show file tree
Hide file tree
Showing 17 changed files with 65 additions and 142 deletions.
2 changes: 1 addition & 1 deletion .changeset/disabled-link-preload.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
"@remix-run/react": patch
---

Add `<link rel="preload">` timeout counter and disabling logic in case preloading is disabled by the user in Firefox. This prevents us from hanging on client-side navigations when we try to preload stylesheets and never receive a `load`/`error` event on the `link` tag.
Skip preloading of stylesheets on client-side route transitions if the browser does not support `<link rel=preload>`. This prevents us from hanging on client-side navigations when we try to preload stylesheets and never receive a `load`/`error` event on the `link` tag.
5 changes: 5 additions & 0 deletions .changeset/giant-crews-care.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/dev": major
---

Remove `devServerBroadcastDelay` config option
5 changes: 5 additions & 0 deletions .changeset/resource-route-boundary-only.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/server-runtime": patch
---

Fix false-positive resource route classification on document requests for routes that only export an `ErrorBoundary`
8 changes: 0 additions & 8 deletions docs/api/conventions.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,6 @@ title: Conventions

[Moved →][moved-4]

### devServerBroadcastDelay

[Moved →][moved-5]

### devServerPort

[Moved →][moved-6]

### ignoredRouteFiles

[Moved →][moved-7]
Expand Down
21 changes: 0 additions & 21 deletions docs/file-conventions/remix-config.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,27 +45,6 @@ The path to the browser build, relative to remix.config.js. Defaults to
The path to a directory Remix can use for caching things in development,
relative to `remix.config.js`. Defaults to `".cache"`.

## devServerBroadcastDelay

<docs-warning>This option is deprecated and will likely be removed in a future
stable release. Enable `v2_dev` to eliminate the race conditions that necessitated
this option.</docs-warning>

The delay, in milliseconds, before the dev server broadcasts a reload event.
There is no delay by default.

For v2, the race conditions that necessitated this option have been eliminated.

## devServerPort

<docs-warning>This option is deprecated and will likely be removed in a future
stable release. Enable `v2_dev` and use [`--port` / `v2_dev.port` option][port]
instead.</docs-warning>

The port number to use for the dev websocket server. Defaults to 8002.

For v2, use [`--port` / `dev.port` option][port].

## ignoredRouteFiles

This is an array of globs (via [minimatch][minimatch]) that Remix will match to
Expand Down
1 change: 0 additions & 1 deletion integration/helpers/cf-template/remix.config.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
/** @type {import('@remix-run/dev').AppConfig} */
export default {
devServerBroadcastDelay: 1000,
ignoredRouteFiles: ["**/.*"],
server: "./server.ts",
serverConditions: ["worker"],
Expand Down
7 changes: 0 additions & 7 deletions integration/helpers/deno-template/remix.config.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,5 @@
/** @type {import('@remix-run/dev').AppConfig} */
export default {
/*
If live reload causes page to re-render without changes (live reload is too fast),
increase the dev server broadcast delay.
If live reload seems slow, try to decrease the dev server broadcast delay.
*/
devServerBroadcastDelay: 300,
ignoredRouteFiles: ["**/.*"],
server: "./server.ts",
serverConditions: ["deno", "worker"],
Expand Down
24 changes: 24 additions & 0 deletions integration/resource-routes-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ test.describe("loader in an app", async () => {
export default () => (
<>
<Link to="/redirect">Redirect</Link>
<Link to="/some-404-path">404 route</Link>
<Form action="/redirect-to" method="post">
<input name="destination" defaultValue="/redirect-destination" />
<button type="submit">Redirect</button>
Expand Down Expand Up @@ -109,6 +110,19 @@ test.describe("loader in an app", async () => {
return json({ ok: true });
}
`,
"app/routes/$.tsx": js`
import { json } from "@remix-run/node";
import { useRouteError } from "@remix-run/react";
export function loader({ request }) {
throw json({ message: new URL(request.url).pathname + ' not found' }, {
status: 404
});
}
export function ErrorBoundary() {
let error = useRouteError();
return <pre>{error.status + ' ' + error.data.message}</pre>;
}
`,
},
});
appFixture = await createAppFixture(fixture, ServerMode.Test);
Expand Down Expand Up @@ -167,6 +181,16 @@ test.describe("loader in an app", async () => {
"Unexpected Server Error\n\nError: Oh noes!"
);
});

test("should let loader throw to it's own boundary without a default export", async ({
page,
}) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/", true);
await app.clickLink("/some-404-path");
let html = await app.getHtml();
expect(html).toMatch("404 /some-404-path not found");
});
}

test("should handle responses returned from resource routes", async ({
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
/** @type {import('@remix-run/dev').AppConfig} */
module.exports = {
devServerBroadcastDelay: 1000,
ignoredRouteFiles: ["**/.*"],
server: "./server.ts",
serverBuildPath: "functions/[[path]].js",
Expand Down
7 changes: 0 additions & 7 deletions packages/remix-dev/__tests__/fixtures/deno/remix.config.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,5 @@
/** @type {import('@remix-run/dev').AppConfig} */
module.exports = {
/*
If live reload causes page to re-render without changes (live reload is too fast),
increase the dev server broadcast delay.
If live reload seems slow, try to decrease the dev server broadcast delay.
*/
devServerBroadcastDelay: 300,
ignoredRouteFiles: ["**/.*"],
server: "./server.ts",
serverConditions: ["deno", "worker"],
Expand Down
1 change: 0 additions & 1 deletion packages/remix-dev/__tests__/readConfig-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ describe("readConfig", () => {
"assetsBuildDirectory": Any<String>,
"cacheDirectory": Any<String>,
"dev": Object {},
"devServerBroadcastDelay": 0,
"entryClientFile": "entry.client.tsx",
"entryClientFilePath": Any<String>,
"entryServerFile": "entry.server.tsx",
Expand Down
47 changes: 4 additions & 43 deletions packages/remix-dev/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,17 +83,6 @@ export interface AppConfig {
*/
dev?: Dev;

/**
* @deprecated
*
* The delay, in milliseconds, before the dev server broadcasts a reload
* event. There is no delay by default.
*
* @deprecated Enable {@link AppConfig.future.v2_dev} to eliminate the race
* conditions that necessitated this option.
*/
devServerBroadcastDelay?: number;

/**
* Additional MDX remark / rehype plugins.
*/
Expand Down Expand Up @@ -252,14 +241,6 @@ export interface RemixConfig {
*/
dev: Dev;

/**
* The delay before the dev (asset) server broadcasts a reload event.
*
* @deprecated Enable {@link RemixConfig.future.v2_dev} to eliminate the race
* conditions that necessitated this option.
*/
devServerBroadcastDelay: number;

/**
* Additional MDX remark / rehype plugins.
*/
Expand Down Expand Up @@ -383,8 +364,8 @@ export async function readConfig(
// shout out to next
// https://github.com/vercel/next.js/blob/b15a976e11bf1dc867c241a4c1734757427d609c/packages/next/server/config.ts#L748-L765
if (process.env.JEST_WORKER_ID) {
// dynamic import does not currently work inside of vm which
// jest relies on so we fall back to require for this case
// dynamic import does not currently work inside vm which
// jest relies on, so we fall back to require for this case
// https://github.com/nodejs/node/issues/35889
appConfigModule = require(configFile);
} else {
Expand Down Expand Up @@ -557,13 +538,6 @@ export async function readConfig(
assetsBuildDirectory
);

if (appConfig.devServerBroadcastDelay) {
devServerBroadcastDelayWarning();
}

// set env variable so un-bundled servers can use it
let devServerBroadcastDelay = appConfig.devServerBroadcastDelay || 0;

let publicPath = addTrailingSlash(appConfig.publicPath || "/build/");

let rootRouteFile = findEntry(appDirectory, "root");
Expand Down Expand Up @@ -615,8 +589,8 @@ export async function readConfig(
}

// Note: When a future flag is removed from here, it should be added to the
// list below so we can let folks know if they have obsolete flags in their
// config. If we ever convert remix.config.js to a TS file so we get proper
// list below, so we can let folks know if they have obsolete flags in their
// config. If we ever convert remix.config.js to a TS file, so we get proper
// typings this won't be necessary anymore.
let future: FutureConfig = {};

Expand Down Expand Up @@ -655,7 +629,6 @@ export async function readConfig(
entryServerFile,
entryServerFilePath,
dev: appConfig.dev ?? {},
devServerBroadcastDelay,
assetsBuildDirectory: absoluteAssetsBuildDirectory,
relativeAssetsBuildDirectory: assetsBuildDirectory,
publicPath,
Expand Down Expand Up @@ -748,15 +721,3 @@ let disjunctionListFormat = new Intl.ListFormat("en", {
style: "long",
type: "disjunction",
});

let devServerBroadcastDelayWarning = () =>
logger.warn(
"The `devServerBroadcastDelay` config option will be removed in v2",
{
details: [
"Enable `v2_dev` to eliminate the race conditions that necessitated this option.",
"-> https://remix.run/docs/en/v1.19.3/pages/v2#devserverbroadcastdelay",
],
key: "devServerBroadcastDelayWarning",
}
);
2 changes: 1 addition & 1 deletion packages/remix-dev/devServer/liveReload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export async function liveReload(
client.send(JSON.stringify(event));
}
});
}, config.devServerBroadcastDelay);
}, 500);
}

function log(message: string) {
Expand Down
65 changes: 23 additions & 42 deletions packages/remix-react/links.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,42 +221,12 @@ export function getKeyedLinksForMatches(
return dedupeLinkDescriptors(descriptors, preloads);
}

let stylesheetPreloadTimeouts = 0;
let isPreloadDisabled = false;

export async function prefetchStyleLinks(
routeModule: RouteModule
): Promise<void> {
if (!routeModule.links) return;
if (!routeModule.links || !isPreloadSupported()) return;
let descriptors = routeModule.links();
if (!descriptors) return;
if (isPreloadDisabled) return;

// If we've hit our timeout 3 times, we may be in firefox with the
// `network.preload` config disabled and we'll _never_ get onload/onerror
// callbacks. Let's try to confirm this with a totally invalid link preload
// which should immediately throw the onerror
if (stylesheetPreloadTimeouts >= 3) {
let linkLoadedOrErrored = await prefetchStyleLink({
rel: "preload",
as: "style",
href: "__remix-preload-detection-404.css",
});
if (linkLoadedOrErrored) {
// If this processed correctly, then our previous timeouts were probably
// legit, reset the counter.
stylesheetPreloadTimeouts = 0;
} else {
// If this bogus preload also times out without an onerror then it's safe
// to assume preloading is disabled and let's just stop trying. This
// _will_ cause FOUC on destination pages but there's nothing we can
// really do there if preloading is disabled since client-side injected
// scripts aren't render blocking. Maybe eventually React's client side
// async component stuff will provide an easier solution here
console.warn("Disabling preload due to lack of browser support");
isPreloadDisabled = true;
}
}

let styleLinks: HtmlLinkDescriptor[] = [];
for (let descriptor of descriptors) {
Expand All @@ -276,12 +246,13 @@ export async function prefetchStyleLinks(
(!link.media || window.matchMedia(link.media).matches) &&
!document.querySelector(`link[rel="stylesheet"][href="${link.href}"]`)
);

await Promise.all(matchingLinks.map(prefetchStyleLink));
}

async function prefetchStyleLink(
descriptor: HtmlLinkDescriptor
): Promise<boolean> {
): Promise<void> {
return new Promise((resolve) => {
let link = document.createElement("link");
Object.assign(link, descriptor);
Expand All @@ -295,20 +266,16 @@ async function prefetchStyleLink(
}
}

// Allow 3s for the link preload to timeout
let timeoutId = setTimeout(() => {
stylesheetPreloadTimeouts++;
link.onload = () => {
removeLink();
resolve(false);
}, 3_000);
resolve();
};

let done = () => {
clearTimeout(timeoutId);
link.onerror = () => {
removeLink();
resolve(true);
resolve();
};
link.onload = done;
link.onerror = done;

document.head.appendChild(link);
});
}
Expand Down Expand Up @@ -554,3 +521,17 @@ function parsePathPatch(href: string) {
if (path.search === undefined) path.search = "";
return path;
}

// Detect if this browser supports <link rel="preload"> (or has it enabled).
// Originally added to handle the firefox `network.preload` config:
// https://bugzilla.mozilla.org/show_bug.cgi?id=1847811
let _isPreloadSupported: boolean | undefined;
function isPreloadSupported(): boolean {
if (_isPreloadSupported !== undefined) {
return _isPreloadSupported;
}
let el: HTMLLinkElement | null = document.createElement("link");
_isPreloadSupported = el.relList.supports("preload");
el = null;
return _isPreloadSupported;
}
3 changes: 2 additions & 1 deletion packages/remix-server-runtime/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ export const createRequestHandler: CreateRequestHandlerFunction = (
}
} else if (
matches &&
matches[matches.length - 1].route.module.default == null
matches[matches.length - 1].route.module.default == null &&
matches[matches.length - 1].route.module.ErrorBoundary == null
) {
response = await handleResourceRequestRR(
serverMode,
Expand Down
1 change: 0 additions & 1 deletion templates/cloudflare-pages/remix.config.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
/** @type {import('@remix-run/dev').AppConfig} */
export default {
devServerBroadcastDelay: 1000,
ignoredRouteFiles: ["**/.*"],
server: "./server.ts",
serverBuildPath: "functions/[[path]].js",
Expand Down
7 changes: 0 additions & 7 deletions templates/deno/remix.config.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,5 @@
/** @type {import('@remix-run/dev').AppConfig} */
module.exports = {
/*
If live reload causes page to re-render without changes (live reload is too fast),
increase the dev server broadcast delay.
If live reload seems slow, try to decrease the dev server broadcast delay.
*/
devServerBroadcastDelay: 300,
ignoredRouteFiles: ["**/.*"],
server: "./server.ts",
serverConditions: ["deno", "worker"],
Expand Down

0 comments on commit 7a88a00

Please sign in to comment.