Skip to content

Commit

Permalink
Merge branch 'dev' into dev
Browse files Browse the repository at this point in the history
  • Loading branch information
19Qingfeng authored May 24, 2023
2 parents f7b4235 + c90c16e commit cc3059e
Show file tree
Hide file tree
Showing 13 changed files with 143 additions and 68 deletions.
5 changes: 5 additions & 0 deletions .changeset/error-response-resource-route.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/server-runtime": patch
---

Properly handle thrown `ErrorResponse` instances inside resource routes
2 changes: 1 addition & 1 deletion .changeset/expose-error-headers.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
"@remix-run/server-runtime": minor
---

Add `errorHeaders` parameter to the leaf `headers()` function to expose headers from thrown responses that bubble up to ancestor route boundaries. If the throwing route contains the boundary, then `errorHeaders` will be the same object as `loaderHeaders`/`actionHeaders` for that route.
Add `errorHeaders` parameter to the leaf `headers()` function to expose headers from thrown responses that bubble up to ancestor route boundaries. If the throwing route contains the boundary, then `errorHeaders` will be the same object as `loaderHeaders`/`actionHeaders` for that route.
7 changes: 7 additions & 0 deletions .changeset/gorgeous-fireants-fry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@remix-run/dev": patch
---

Fix Tailwind performance issue when `postcss.config.js` contains `plugins: { tailwindcss: {} }` and `remix.config.js` contains both `tailwind: true` and `postcss: true`.

Note that this was _not_ an issue when the plugin function had been explicitly called, i.e. `plugins: [tailwindcss()]`. Remix avoids adding the Tailwind plugin to PostCSS if it's already present but we were failing to detect when the plugin function hadn't been called — either because the plugin function itself had been passed, i.e. `plugins: [require('tailwindcss')]`, or the plugin config object syntax had been used, i.e. `plugins: { tailwindcss: {} }`.
9 changes: 9 additions & 0 deletions .changeset/great-geese-compare.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@remix-run/react": minor
"@remix-run/dev": patch
---

Faster server export removal for routes when `unstable_dev` is enabled.

Also, only render modulepreloads on SSR.
Do not render modulepreloads when hydrated.
6 changes: 3 additions & 3 deletions .changeset/headers-args-type.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@
Add `HeadersArgs` type to be consistent with loaders/actions/meta and allows for using a `function` declaration in addition to an arrow function expression

```tsx
import type { HeadersArgs } from '@remix-run/node'; // or cloudflare/deno
import type { HeadersArgs } from "@remix-run/node"; // or cloudflare/deno

export function headers({ loaderHeaders }: HeadersArgs) {
return {
"x-my-custom-thing": loaderHeaders.get("x-my-custom-thing") || "fallback"
}
"x-my-custom-thing": loaderHeaders.get("x-my-custom-thing") || "fallback",
};
}
```
7 changes: 4 additions & 3 deletions .changeset/lemon-beers-fetch.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,20 @@
Force Typescript to simplify type produced by `Serialize`.

As a result, the following types and functions have simplified return types:

- SerializeFrom
- useLoaderData
- useActionData
- useFetcher

```ts
type Data = { hello: string; when: Date }
type Data = { hello: string; when: Date };

// BEFORE
type Unsimplified = SerializeFrom<Data>
type Unsimplified = SerializeFrom<Data>;
// ^? SerializeObject<UndefinedToOptional<{ hello: string; when: Date }>>

// AFTER
type Simplified = SerializeFrom<Data>
type Simplified = SerializeFrom<Data>;
// ^? { hello: string; when: string }
```
35 changes: 35 additions & 0 deletions integration/resource-routes-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,13 @@ import { PlaywrightFixture } from "./helpers/playwright-fixture";
test.describe("loader in an app", async () => {
let appFixture: AppFixture;
let fixture: Fixture;
let _consoleError: typeof console.error;

let SVG_CONTENTS = `<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 100 100" fill="none" stroke="#000" stroke-width="4" aria-label="Chicken"><path d="M48.1 34C22.1 32 1.4 51 2.5 67.2c1.2 16.1 19.8 17 29 17.8H89c15.7-6.6 6.3-18.9.3-20.5A28 28 0 0073 41.7c-.5-7.2 3.4-11.6 6.9-15.3 8.5 2.6 8-8 .8-7.2.6-6.5-12.3-5.9-6.7 2.7l-3.7 5c-6.9 5.4-10.9 5.1-22.2 7zM48.1 34c-38 31.9 29.8 58.4 25 7.7M70.3 26.9l5.4 4.2"/></svg>`;

test.beforeAll(async () => {
_consoleError = console.error;
console.error = () => {};
fixture = await createFixture({
future: { v2_routeConvention: true },
files: {
Expand All @@ -25,6 +28,9 @@ test.describe("loader in an app", async () => {
<input name="destination" defaultValue="/redirect-destination" />
<button type="submit">Redirect</button>
</Form>
<Form action="/no-action" method="post">
<button type="submit">Submit to no action route</button>
</Form>
</>
)
`,
Expand Down Expand Up @@ -94,13 +100,20 @@ test.describe("loader in an app", async () => {
throw { but: 'why' };
}
`,
"app/routes/no-action.jsx": js`
import { json } from "@remix-run/node";
export let loader = () => {
return json({ ok: true });
}
`,
},
});
appFixture = await createAppFixture(fixture, ServerMode.Test);
});

test.afterAll(() => {
appFixture.close();
console.error = _consoleError;
});

test.describe("with JavaScript", () => {
Expand Down Expand Up @@ -190,6 +203,28 @@ test.describe("loader in an app", async () => {
"Unexpected Server Error\n\n[object Object]"
);
});

test("should handle ErrorResponses thrown from resource routes on document requests", async () => {
let res = await fixture.postDocument("/no-action", new FormData());
expect(res.status).toBe(405);
expect(res.statusText).toBe("Method Not Allowed");
expect(await res.text()).toBe('{"message":"Unexpected Server Error"}');
});

test("should handle ErrorResponses thrown from resource routes on client submissions", async ({
page,
}) => {
let logs: string[] = [];
page.on("console", (msg) => logs.push(msg.text()));
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/");
await app.clickSubmitButton("/no-action");
let html = await app.getHtml();
expect(html).toMatch("Application Error");
expect(logs[0]).toContain(
'Route "routes/no-action" does not have an action'
);
});
});

test.describe("Development server", async () => {
Expand Down
36 changes: 10 additions & 26 deletions packages/remix-dev/compiler/js/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { type Manifest } from "../../manifest";
import { getAppDependencies } from "../../dependencies";
import { loaders } from "../utils/loaders";
import { browserRouteModulesPlugin } from "./plugins/routes";
import { browserRouteModulesPlugin as browserRouteModulesPlugin_v2 } from "./plugins/routes_unstable";
import { cssFilePlugin } from "../plugins/cssImports";
import { absoluteCssUrlsPlugin } from "../plugins/absoluteCssUrlsPlugin";
import { deprecatedRemixPackagePlugin } from "../plugins/deprecatedRemixPackage";
Expand Down Expand Up @@ -78,29 +77,12 @@ const createEsbuildConfig = (
"entry.client": ctx.config.entryClientFilePath,
};

let routeModulePaths = new Map<string, string>();
for (let id of Object.keys(ctx.config.routes)) {
entryPoints[id] = ctx.config.routes[id].file;
if (ctx.config.future.unstable_dev) {
// In V2 we are doing AST transforms to remove server code, this means we
// have to re-map all route modules back to the same module in the graph
// otherwise we will have duplicate modules in the graph. We have to resolve
// the path as we get the relative for the entrypoint and absolute for imports
// from other modules.
routeModulePaths.set(
ctx.config.routes[id].file,
ctx.config.routes[id].file
);
routeModulePaths.set(
path.resolve(ctx.config.appDirectory, ctx.config.routes[id].file),
ctx.config.routes[id].file
);
} else {
// All route entry points are virtual modules that will be loaded by the
// browserEntryPointsPlugin. This allows us to tree-shake server-only code
// that we don't want to run in the browser (i.e. action & loader).
entryPoints[id] += "?browser";
}
// All route entry points are virtual modules that will be loaded by the
// browserEntryPointsPlugin. This allows us to tree-shake server-only code
// that we don't want to run in the browser (i.e. action & loader).
entryPoints[id] += "?browser";
}

if (
Expand Down Expand Up @@ -133,16 +115,18 @@ const createEsbuildConfig = (
}

let plugins: esbuild.Plugin[] = [
browserRouteModulesPlugin(ctx, /\?browser$/),
deprecatedRemixPackagePlugin(ctx),
cssModulesPlugin(ctx, { outputCss: false }),
vanillaExtractPlugin(ctx, { outputCss: false }),
cssSideEffectImportsPlugin(ctx),
cssSideEffectImportsPlugin(ctx, {
hmr:
ctx.options.mode === "development" &&
ctx.config.future.unstable_dev !== false,
}),
cssFilePlugin(ctx),
absoluteCssUrlsPlugin(),
externalPlugin(/^https?:\/\//, { sideEffects: false }),
ctx.config.future.unstable_dev
? browserRouteModulesPlugin_v2(ctx, routeModulePaths)
: browserRouteModulesPlugin(ctx, /\?browser$/),
mdxPlugin(ctx),
emptyModulesPlugin(ctx, /\.server(\.[jt]sx?)?$/),
NodeModulesPolyfillPlugin(),
Expand Down
30 changes: 18 additions & 12 deletions packages/remix-dev/compiler/plugins/cssSideEffectImports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ import { parse, type ParserOptions } from "@babel/parser";
import traverse from "@babel/traverse";
import generate from "@babel/generator";

import type { RemixConfig } from "../../config";
import type { Options } from "../options";
import { getPostcssProcessor } from "../utils/postcss";
import { applyHMR } from "../js/plugins/hmr";
import type { Context } from "../context";

const pluginName = "css-side-effects-plugin";
const namespace = `${pluginName}-ns`;
Expand Down Expand Up @@ -44,17 +44,14 @@ const loaderForExtension: Record<Extension, Loader> = {
* to the CSS bundle. This is primarily designed to support packages that
* import plain CSS files directly within JS files.
*/
export const cssSideEffectImportsPlugin = ({
config,
options,
}: {
config: RemixConfig;
options: Options;
}): Plugin => {
export const cssSideEffectImportsPlugin = (
ctx: Context,
{ hmr = false } = {}
): Plugin => {
return {
name: pluginName,
setup: async (build) => {
let postcssProcessor = await getPostcssProcessor({ config });
let postcssProcessor = await getPostcssProcessor(ctx);

build.onLoad(
{ filter: allJsFilesFilter, namespace: "file" },
Expand All @@ -69,6 +66,15 @@ export const cssSideEffectImportsPlugin = ({
let loader = loaderForExtension[path.extname(args.path) as Extension];
let contents = addSuffixToCssSideEffectImports(loader, code);

if (hmr) {
contents = await applyHMR(
contents,
args,
ctx.config,
!!build.initialOptions.sourcemap
);
}

return {
contents,
loader,
Expand All @@ -94,7 +100,7 @@ export const cssSideEffectImportsPlugin = ({
}

return {
path: path.relative(config.rootDirectory, resolvedPath),
path: path.relative(ctx.config.rootDirectory, resolvedPath),
namespace,
};
}
Expand All @@ -108,7 +114,7 @@ export const cssSideEffectImportsPlugin = ({
await postcssProcessor.process(contents, {
from: args.path,
to: args.path,
map: options.sourcemap,
map: ctx.options.sourcemap,
})
).css;
}
Expand Down
11 changes: 8 additions & 3 deletions packages/remix-dev/compiler/utils/postcss.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export async function loadPostcssPlugins({

if (config.tailwind) {
let tailwindPlugin = await loadTailwindPlugin(config);
if (tailwindPlugin && !hasTailwindPlugin(plugins)) {
if (tailwindPlugin && !hasTailwindPlugin(plugins, tailwindPlugin)) {
plugins.push(tailwindPlugin);
}
}
Expand Down Expand Up @@ -94,10 +94,15 @@ export async function getPostcssProcessor({
return processor;
}

function hasTailwindPlugin(plugins: Array<AcceptedPlugin>) {
function hasTailwindPlugin(
plugins: Array<AcceptedPlugin>,
tailwindPlugin: AcceptedPlugin
) {
return plugins.some(
(plugin) =>
"postcssPlugin" in plugin && plugin.postcssPlugin === "tailwindcss"
plugin === tailwindPlugin ||
(typeof plugin === "function" && plugin.name === "tailwindcss") ||
("postcssPlugin" in plugin && plugin.postcssPlugin === "tailwindcss")
);
}

Expand Down
11 changes: 3 additions & 8 deletions packages/remix-react/components.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1087,13 +1087,8 @@ import(${JSON.stringify(manifest.entry.module)});`;

let preloads = isHydrated ? [] : manifest.entry.imports.concat(routePreloads);

return (
return isHydrated ? null : (
<>
<link
rel="modulepreload"
href={manifest.url}
crossOrigin={props.crossOrigin}
/>
<link
rel="modulepreload"
href={manifest.entry.module}
Expand All @@ -1107,8 +1102,8 @@ import(${JSON.stringify(manifest.entry.module)});`;
crossOrigin={props.crossOrigin}
/>
))}
{!isHydrated && initialScripts}
{!isHydrated && deferredScripts}
{initialScripts}
{deferredScripts}
</>
);
}
Expand Down
6 changes: 5 additions & 1 deletion packages/remix-server-runtime/serialize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,11 @@ type SerializeObject<T extends object> = {

// prettier-ignore
type SerializeDeferred<T extends Record<string, unknown>> = {
[k in keyof T as T[k] extends Promise<unknown> ? k : T[k] extends NonJsonPrimitive ? never : k]:
[k in keyof T as
T[k] extends Promise<unknown> ? k :
T[k] extends NonJsonPrimitive ? never :
k
]:
T[k] extends Promise<infer U>
? Promise<Serialize<U>> extends never ? "wtf" : Promise<Serialize<U>>
: Serialize<T[k]> extends never ? k : Serialize<T[k]>;
Expand Down
Loading

0 comments on commit cc3059e

Please sign in to comment.