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

perf(dev): faster server export removal for routes #6455

Merged
merged 3 commits into from
May 23, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.
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: 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