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

refactor(remix-dev/vite): resolve absolute serverBuildDirectory #8542

Merged
merged 2 commits into from
Jan 18, 2024
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
13 changes: 6 additions & 7 deletions packages/remix-dev/vite/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,18 +139,21 @@ async function getServerBuilds({
}
serverBundlesManifest.routeIdToServerBundleId[route.id] = bundleId;

let serverBundleDirectory = path.join(serverBuildDirectory, bundleId);
let relativeServerBundleDirectory = path.relative(
rootDirectory,
path.join(serverBuildDirectory, bundleId)
);
let serverBuildConfig = serverBuildConfigByBundleId.get(bundleId);
if (!serverBuildConfig) {
serverBundlesManifest.serverBundles[bundleId] = {
id: bundleId,
file: normalizePath(
path.join(serverBundleDirectory, serverBuildFile)
path.join(relativeServerBundleDirectory, serverBuildFile)
Copy link
Member Author

Choose a reason for hiding this comment

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

This file is a case where we want it to be relative since it's a root-relative manifest file.

),
};
serverBuildConfig = {
routes: {},
serverBuildDirectory: serverBundleDirectory,
serverBuildDirectory: relativeServerBundleDirectory,
};
serverBuildConfigByBundleId.set(bundleId, serverBuildConfig);
}
Expand Down Expand Up @@ -264,10 +267,6 @@ export async function build(
serverBuildFile,
} = remixConfig;

// Should this already be absolute on the resolved config object?
// In the meantime, we make it absolute before passing to adapter hooks
serverBuildDirectory = path.resolve(root, serverBuildDirectory);

await remixConfig.adapter?.buildEnd?.({
// Since this is public API, these properties need to mirror the options
// passed to the Remix plugin. This means we need to translate our internal
Expand Down
50 changes: 30 additions & 20 deletions packages/remix-dev/vite/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ import colors from "picocolors";

import { type ConfigRoute, type RouteManifest } from "../config/routes";
import {
type AppConfig as RemixUserConfig,
type RemixConfig as ResolvedRemixConfig,
resolveConfig,
type AppConfig as RemixEsbuildUserConfig,
type RemixConfig as ResolvedRemixEsbuildConfig,
resolveConfig as resolveRemixEsbuildConfig,
} from "../config";
import { type Manifest } from "../manifest";
import invariant from "../invariant";
Expand All @@ -35,17 +35,19 @@ import { removeExports } from "./remove-exports";
import { replaceImportSpecifier } from "./replace-import-specifier";
import { importViteEsmSync, preloadViteEsm } from "./import-vite-esm-sync";

const supportedRemixConfigKeys = [
const supportedRemixEsbuildConfigKeys = [
"appDirectory",
"assetsBuildDirectory",
"future",
"ignoredRouteFiles",
"publicPath",
"routes",
"serverModuleFormat",
] as const satisfies ReadonlyArray<keyof RemixUserConfig>;
type SupportedRemixConfigKey = typeof supportedRemixConfigKeys[number];
type SupportedRemixConfig = Pick<RemixUserConfig, SupportedRemixConfigKey>;
] as const satisfies ReadonlyArray<keyof RemixEsbuildUserConfig>;
type SupportedRemixEsbuildUserConfig = Pick<
RemixEsbuildUserConfig,
typeof supportedRemixEsbuildConfigKeys[number]
>;

const SERVER_ONLY_ROUTE_EXPORTS = ["loader", "action", "headers"];
const CLIENT_ROUTE_EXPORTS = [
Expand All @@ -64,17 +66,17 @@ const CLIENT_ROUTE_QUERY_STRING = "?client-route";

// We need to provide different JSDoc comments in some cases due to differences
// between the Remix config and the Vite plugin.
type RemixConfigJsdocOverrides = {
type RemixEsbuildUserConfigJsdocOverrides = {
/**
* The path to the browser build, relative to the project root. Defaults to
* `"build/client"`.
*/
assetsBuildDirectory?: SupportedRemixConfig["assetsBuildDirectory"];
assetsBuildDirectory?: SupportedRemixEsbuildUserConfig["assetsBuildDirectory"];
/**
* The URL prefix of the browser build with a trailing slash. Defaults to
* `"/"`. This is the path the browser will use to find assets.
*/
publicPath?: SupportedRemixConfig["publicPath"];
publicPath?: SupportedRemixEsbuildUserConfig["publicPath"];
};

// Only expose a subset of route properties to the "serverBundles" function
Expand Down Expand Up @@ -122,8 +124,11 @@ export type VitePluginAdapter = (args: {
remixConfig: VitePluginConfig;
}) => Adapter | Promise<Adapter>;

export type VitePluginConfig = RemixConfigJsdocOverrides &
Omit<SupportedRemixConfig, keyof RemixConfigJsdocOverrides> & {
export type VitePluginConfig = RemixEsbuildUserConfigJsdocOverrides &
Omit<
SupportedRemixEsbuildUserConfig,
keyof RemixEsbuildUserConfigJsdocOverrides
> & {
/**
* A function for adapting the build output and/or development environment
* for different hosting providers.
Expand Down Expand Up @@ -166,7 +171,7 @@ type BuildEndArgs = Pick<
type BuildEndHook = (args: BuildEndArgs) => void | Promise<void>;

export type ResolvedVitePluginConfig = Pick<
ResolvedRemixConfig,
ResolvedRemixEsbuildConfig,
| "appDirectory"
| "rootDirectory"
| "assetsBuildDirectory"
Expand Down Expand Up @@ -453,14 +458,19 @@ export const remixVitePlugin: RemixVitePlugin = (remixUserConfig = {}) => {

let isSpaMode = mergedRemixConfig.unstable_ssr === false;

let resolvedRemixConfig = await resolveConfig(
pick(mergedRemixConfig, supportedRemixConfigKeys),
{
rootDirectory,
isSpaMode,
}
let resolvedRemixEsbuildConfig = await resolveRemixEsbuildConfig(
pick(mergedRemixConfig, supportedRemixEsbuildConfigKeys),
{ rootDirectory, isSpaMode }
);

let resolvedRemixConfig = {
...resolvedRemixEsbuildConfig,
serverBuildDirectory: path.resolve(
rootDirectory,
mergedRemixConfig.serverBuildDirectory
),
};

// Only select the Remix config options that the Vite plugin uses
let {
appDirectory,
Expand Down Expand Up @@ -1063,7 +1073,7 @@ export const remixVitePlugin: RemixVitePlugin = (remixUserConfig = {}) => {

if (remixConfig.isSpaMode) {
await handleSpaMode(
path.join(rootDirectory, serverBuildDirectory),
serverBuildDirectory,
serverBuildFile,
assetsBuildDirectory,
viteConfig
Expand Down