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

feat(dev): pretty logging for unstable_dev #6496

Closed
wants to merge 4 commits into from
Closed
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
17 changes: 10 additions & 7 deletions packages/remix-dev/cli/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ import runCodemod from "../codemod";
import { CodemodError } from "../codemod/utils/error";
import { TaskError } from "../codemod/utils/task";
import { transpile as convertFileToJS } from "./useJavascript";
import { warnOnce } from "../warnOnce";
import type { Options } from "../compiler/options";
import { logger } from "../tux/logger";

export async function create({
appTemplate,
Expand Down Expand Up @@ -173,7 +173,7 @@ export async function build(
let options: Options = {
mode,
sourcemap,
onWarning: warnOnce,
logger,
};
if (mode === "development" && config.future.unstable_dev) {
let origin = await resolveDevOrigin(config);
Expand Down Expand Up @@ -221,12 +221,15 @@ export async function dev(
tlsCert?: string;
} = {}
) {
// clear screen
process.stdout.write("\x1Bc");

// TODO: statically get version
let version = "1.17.0";
console.log(`\n 💿 remix dev v${version}\n`);

if (process.env.NODE_ENV && process.env.NODE_ENV !== "development") {
console.warn(
`Forcing NODE_ENV to be 'development'. Was: ${JSON.stringify(
process.env.NODE_ENV
)}`
);
logger.warn(`overriding NODE_ENV=${process.env.NODE_ENV} to development`);
}
process.env.NODE_ENV = "development";
if (flags.debug) inspector.open();
Expand Down
5 changes: 2 additions & 3 deletions packages/remix-dev/compiler/js/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ const createEsbuildConfig = (
let packageName = getNpmPackageName(args.path);
let pkgManager = detectPackageManager() ?? "npm";
if (
ctx.options.onWarning &&
!isNodeBuiltIn(packageName) &&
!/\bnode_modules\b/.test(args.importer) &&
// Silence spurious warnings when using Yarn PnP. Yarn PnP doesn’t use
Expand All @@ -159,12 +158,12 @@ const createEsbuildConfig = (
try {
require.resolve(args.path);
} catch (error: unknown) {
ctx.options.onWarning(
ctx.options.logger.warn(
`The path "${args.path}" is imported in ` +
`${path.relative(process.cwd(), args.importer)} but ` +
`"${args.path}" was not found in your node_modules. ` +
`Did you forget to install it?`,
args.path
{ key: args.path }
);
}
}
Expand Down
4 changes: 3 additions & 1 deletion packages/remix-dev/compiler/options.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import type { Logger } from "../tux/logger";

type Mode = "development" | "production" | "test";

export type Options = {
mode: Mode;
sourcemap: boolean;
onWarning?: (message: string, key: string) => void;
logger: Logger;

// TODO: required in v2
devOrigin?: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export function deprecatedRemixPackagePlugin(ctx: Context): Plugin {
`underlying \`@remix-run/*\` package. ` +
`Run \`npx @remix-run/dev@latest codemod replace-remix-magic-imports\` ` +
`to automatically migrate your code.`;
ctx.options.onWarning?.(warningMessage, importer);
ctx.options.logger.warn(warningMessage, { key: importer });
}
return undefined;
});
Expand Down
35 changes: 14 additions & 21 deletions packages/remix-dev/compiler/server/plugins/bareImports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ import type { Context } from "../../context";
* This includes externalizing for node based platforms, and bundling for single file
* environments such as cloudflare.
*/
export function serverBareModulesPlugin({ config, options }: Context): Plugin {
export function serverBareModulesPlugin(ctx: Context): Plugin {
// Resolve paths according to tsconfig paths property
let matchPath = config.tsconfigPath
? createMatchPath(config.tsconfigPath)
let matchPath = ctx.config.tsconfigPath
? createMatchPath(ctx.config.tsconfigPath)
: undefined;
function resolvePath(id: string) {
if (!matchPath) {
Expand Down Expand Up @@ -76,7 +76,6 @@ export function serverBareModulesPlugin({ config, options }: Context): Plugin {

// Warn if we can't find an import for a package.
if (
options.onWarning &&
!isNodeBuiltIn(packageName) &&
!/\bnode_modules\b/.test(importer) &&
// Silence spurious warnings when using Yarn PnP. Yarn PnP doesn’t use
Expand All @@ -88,21 +87,21 @@ export function serverBareModulesPlugin({ config, options }: Context): Plugin {
try {
require.resolve(path, { paths: [importer] });
} catch (error: unknown) {
options.onWarning(
ctx.options.logger.warn(
`The path "${path}" is imported in ` +
`${relative(process.cwd(), importer)} but ` +
`"${path}" was not found in your node_modules. ` +
`Did you forget to install it?`,
path
{ key: path }
);
}
}

if (config.serverDependenciesToBundle === "all") {
if (ctx.config.serverDependenciesToBundle === "all") {
return undefined;
}

for (let pattern of config.serverDependenciesToBundle) {
for (let pattern of ctx.config.serverDependenciesToBundle) {
// bundle it if the path matches the pattern
if (
typeof pattern === "string" ? path === pattern : pattern.test(path)
Expand All @@ -112,17 +111,11 @@ export function serverBareModulesPlugin({ config, options }: Context): Plugin {
}

if (
options.onWarning &&
!isNodeBuiltIn(packageName) &&
kind !== "dynamic-import" &&
config.serverPlatform === "node"
ctx.config.serverPlatform === "node"
) {
warnOnceIfEsmOnlyPackage(
packageName,
path,
importer,
options.onWarning
);
warnOnceIfEsmOnlyPackage(ctx, packageName, path, importer);
}

// Externalize everything else if we've gotten here.
Expand Down Expand Up @@ -151,10 +144,10 @@ function isBareModuleId(id: string): boolean {
}

function warnOnceIfEsmOnlyPackage(
ctx: Context,
packageName: string,
fullImportPath: string,
importer: string,
onWarning: (msg: string, key: string) => void
importer: string
) {
try {
let packageDir = resolveModuleBasePath(
Expand All @@ -165,7 +158,7 @@ function warnOnceIfEsmOnlyPackage(
let packageJsonFile = path.join(packageDir, "package.json");

if (!fs.existsSync(packageJsonFile)) {
console.log(packageJsonFile, `does not exist`);
ctx.options.logger.warn(`${packageJsonFile} does not exist`);
return;
}
let pkg = JSON.parse(fs.readFileSync(packageJsonFile, "utf-8"));
Expand All @@ -187,10 +180,10 @@ function warnOnceIfEsmOnlyPackage(
}

if (isEsmOnly) {
onWarning(
ctx.options.logger.warn(
`${packageName} is possibly an ESM only package and should be bundled with ` +
`"serverDependenciesToBundle" in remix.config.js.`,
packageName + ":esm-only"
{ key: packageName + ":esm-only" }
);
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/remix-dev/compiler/watch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export async function watch(
pollInterval: 100,
},
})
.on("error", (error) => console.error(error))
.on("error", (error) => ctx.options.logger.error(String(error)))
.on("change", async (file) => {
onFileChanged?.(file);
await rebuild();
Expand Down
102 changes: 65 additions & 37 deletions packages/remix-dev/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import fse from "fs-extra";
import getPort from "get-port";
import NPMCliPackageJson from "@npmcli/package-json";
import { coerce } from "semver";
import pc from "picocolors";

import type { RouteManifest, DefineRoutesFunction } from "./config/routes";
import { defineRoutes } from "./config/routes";
Expand All @@ -14,6 +15,7 @@ import { serverBuildVirtualModule } from "./compiler/server/virtualModules";
import { flatRoutes } from "./config/flat-routes";
import { detectPackageManager } from "./cli/detectPackageManager";
import { warnOnce } from "./warnOnce";
import { logger } from "./tux/logger";

export interface RemixMdxConfig {
rehypePlugins?: any[];
Expand Down Expand Up @@ -430,19 +432,19 @@ export async function readConfig(
}

if (!appConfig.future?.v2_errorBoundary) {
warnOnce(errorBoundaryWarning, "v2_errorBoundary");
logger.warn(...errorBoundaryWarning);
}

if (!appConfig.future?.v2_normalizeFormMethod) {
warnOnce(formMethodWarning, "v2_normalizeFormMethod");
logger.warn(...formMethodWarning);
}

if (!appConfig.future?.v2_meta) {
warnOnce(metaWarning, "v2_meta");
logger.warn(...metaWarning);
}

if (!appConfig.future?.v2_headers) {
warnOnce(headersWarning, "v2_headers");
logger.warn(...headersWarning);
}

let isCloudflareRuntime = ["cloudflare-pages", "cloudflare-workers"].includes(
Expand All @@ -462,7 +464,7 @@ export async function readConfig(
let serverMinify = appConfig.serverMinify;

if (!appConfig.serverModuleFormat) {
warnOnce(serverModuleFormatWarning, "serverModuleFormatWarning");
logger.warn(...serverModuleFormatWarning);
}

let serverModuleFormat = appConfig.serverModuleFormat || "cjs";
Expand Down Expand Up @@ -690,7 +692,7 @@ export async function readConfig(
if (appConfig.future?.v2_routeConvention) {
routesConvention = flatRoutes;
} else {
warnOnce(flatRoutesWarning, "v2_routeConvention");
logger.warn(...flatRoutesWarning);
routesConvention = defineConventionalRoutes;
}

Expand Down Expand Up @@ -884,6 +886,23 @@ let disjunctionListFormat = new Intl.ListFormat("en", {
type: "disjunction",
});

let futureWarn = (
message: string,
options: {
flag: string;
link: string;
}
): Parameters<typeof logger.warn> => [
pc.yellow("future") + " " + message,
{
details: [
`You can use the \`${options.flag}\` future flag to opt-in early`,
`-> ${options.link}`,
],
key: options.flag,
},
];

export let browserBuildDirectoryWarning =
"⚠️ REMIX FUTURE CHANGE: The `browserBuildDirectory` config option will be removed in v2. " +
"Use `assetsBuildDirectory` instead. " +
Expand All @@ -902,39 +921,48 @@ export let serverBuildTargetWarning =
"For instructions on making this change see " +
"https://remix.run/docs/en/v1.15.0/pages/v2#serverbuildtarget";

export const serverModuleFormatWarning =
"⚠️ REMIX FUTURE CHANGE: The `serverModuleFormat` config default option will be changing in v2 " +
"from `cjs` to `esm`. You can prepare for this change by explicitly specifying `serverModuleFormat: 'cjs'`. " +
"For instructions on making this change see " +
"https://remix.run/docs/en/v1.16.0/pages/v2#servermoduleformat";
const serverModuleFormatWarning = futureWarn(
"The `serverModuleFormat` config default option will be changing in v2",
{
flag: "TODO",
// "from `cjs` to `esm`. You can prepare for this change by explicitly specifying `serverModuleFormat: 'cjs'`. ";
link: "https://remix.run/docs/en/v1.16.0/pages/v2#servermoduleformat",
}
);

export let flatRoutesWarning =
"⚠️ REMIX FUTURE CHANGE: The route file convention is changing in v2. " +
"You can prepare for this change at your convenience with the `v2_routeConvention` future flag. " +
"For instructions on making this change see " +
"https://remix.run/docs/en/v1.15.0/pages/v2#file-system-route-convention";
const flatRoutesWarning = futureWarn(
"The route file convention is changing in v2",
{
flag: "v2_routeConvention",
link: "https://remix.run/docs/en/v1.15.0/pages/v2#file-system-route-convention",
}
);

export const errorBoundaryWarning =
"⚠️ REMIX FUTURE CHANGE: The behaviors of `CatchBoundary` and `ErrorBoundary` are changing in v2. " +
"You can prepare for this change at your convenience with the `v2_errorBoundary` future flag. " +
"For instructions on making this change see " +
"https://remix.run/docs/en/v1.15.0/pages/v2#catchboundary-and-errorboundary";
const errorBoundaryWarning = futureWarn(
"The behaviors of `CatchBoundary` and `ErrorBoundary` are changing in v2",
{
flag: "v2_errorBoundary",
link: "https://remix.run/docs/en/v1.15.0/pages/v2#catchboundary-and-errorboundary",
}
);

export const formMethodWarning =
"⚠️ REMIX FUTURE CHANGE: APIs that provide `formMethod` will be changing in v2. " +
"All values will be uppercase (GET, POST, etc.) instead of lowercase (get, post, etc.) " +
"You can prepare for this change at your convenience with the `v2_normalizeFormMethod` future flag. " +
"For instructions on making this change see " +
"https://remix.run/docs/en/v1.15.0/pages/v2#formMethod";
const formMethodWarning = futureWarn(
"APIs that provide `formMethod` will be changing in v2",
{
flag: "v2_normalizeFormMethod",
link: "https://remix.run/docs/en/v1.15.0/pages/v2#formMethod",
}
);

export const metaWarning =
"⚠️ REMIX FUTURE CHANGE: The route `meta` export signature is changing in v2. " +
"You can prepare for this change at your convenience with the `v2_meta` future flag. " +
"For instructions on making this change see " +
"https://remix.run/docs/en/v1.15.0/pages/v2#meta";
const metaWarning = futureWarn(
"route `meta` export signature is changing in v2",
{
flag: "v2_meta",
link: "https://remix.run/docs/en/v1.15.0/pages/v2#meta",
}
);

export const headersWarning =
"⚠️ REMIX FUTURE CHANGE: The route `headers` export behavior is changing in v2. " +
"You can prepare for this change at your convenience with the `v2_headers` future flag. " +
"For instructions on making this change see " +
"https://remix.run/docs/en/v1.17.0/pages/v2#route-headers";
const headersWarning = futureWarn("`headers` export is changing in v2", {
flag: "v2_headers",
link: "https://remix.run/docs/en/v1.17.0/pages/v2#route-headers",
});
4 changes: 2 additions & 2 deletions packages/remix-dev/devServer/liveReload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import WebSocket from "ws";

import { watch } from "../compiler";
import type { RemixConfig } from "../config";
import { warnOnce } from "../warnOnce";
import { logger } from "../tux/logger";

const relativePath = (file: string) => path.relative(process.cwd(), file);

Expand Down Expand Up @@ -44,7 +44,7 @@ export async function liveReload(config: RemixConfig) {
options: {
mode: "development",
sourcemap: true,
onWarning: warnOnce,
logger,
},
},
{
Expand Down
3 changes: 2 additions & 1 deletion packages/remix-dev/devServer/serve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import os from "os";
import { loadEnv } from "../devServer_unstable/env";
import { liveReload } from "./liveReload";
import type { RemixConfig } from "../config";
import { logger } from "../tux/logger";

function purgeAppRequireCache(buildPath: string) {
for (let key in require.cache) {
Expand Down Expand Up @@ -36,7 +37,7 @@ export async function serve(config: RemixConfig, portPreference?: number) {
// eslint-disable-next-line @typescript-eslint/consistent-type-imports
let express = tryImport("express") as typeof import("express");

await loadEnv(config.rootDirectory);
await loadEnv(config.rootDirectory, { logger });

let port = await getPort({
port: portPreference
Expand Down
Loading