Skip to content

Commit

Permalink
fix(dev): remove server bare import package check (#7214)
Browse files Browse the repository at this point in the history
  • Loading branch information
markdalgleish authored Aug 22, 2023
1 parent 5de3187 commit cdd6927
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 117 deletions.
73 changes: 0 additions & 73 deletions integration/compiler-test.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
import path from "node:path";
import fse from "fs-extra";
import { test, expect } from "@playwright/test";
import { PassThrough } from "node:stream";

import {
createFixture,
createAppFixture,
js,
json,
createFixtureProject,
css,
} from "./helpers/create-fixture.js";
import type { Fixture, AppFixture } from "./helpers/create-fixture.js";
Expand Down Expand Up @@ -310,75 +308,4 @@ test.describe("compiler", () => {
expect(cssFile).toBeTruthy();
expect(fontFile).toBeTruthy();
});

test.describe("serverBareModulesPlugin", () => {
let ogConsole: typeof global.console;
test.beforeEach(() => {
ogConsole = global.console;
// @ts-ignore
global.console = {
log() {},
warn() {},
error() {},
};
});
test.afterEach(() => {
global.console = ogConsole;
});
test("warns when a module isn't installed", async () => {
let buildOutput: string;
let buildStdio = new PassThrough();

await expect(() =>
createFixtureProject({
buildStdio,
files: {
"app/routes/_index.tsx": js`
import { json } from "@remix-run/node";
import { useLoaderData } from "@remix-run/react";
import notInstalledMain from "some-not-installed-module";
import { notInstalledSub } from "some-not-installed-module/sub";
export function loader() {
return json({ main: notInstalledMain(), sub: notInstalledSub() });
}
export default function Index() {
let data = useLoaderData();
return null;
}
`,
},
})
).rejects.toThrowError("Build failed, check the output above");

let chunks: Buffer[] = [];
buildOutput = await new Promise<string>((resolve, reject) => {
buildStdio.on("error", (error) => {
reject(error);
});
buildStdio.on("data", (chunk) => {
chunks.push(Buffer.from(chunk));
});
buildStdio.on("end", () => {
resolve(Buffer.concat(chunks).toString("utf8"));
});
});

let importer = path.join("app", "routes", "_index.tsx");

expect(buildOutput).toContain(
`could not resolve "some-not-installed-module"`
);
expect(buildOutput).toContain(
`You imported "some-not-installed-module" in ${importer},`
);
expect(buildOutput).toContain(
`could not resolve "some-not-installed-module/sub"`
);
expect(buildOutput).toContain(
`You imported "some-not-installed-module/sub" in ${importer},`
);
});
});
});
45 changes: 1 addition & 44 deletions packages/remix-dev/compiler/server/plugins/bareImports.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { isAbsolute, relative } from "node:path";
import { builtinModules } from "node:module";
import { isAbsolute } from "node:path";
import type { Plugin } from "esbuild";

import {
Expand All @@ -8,7 +7,6 @@ import {
} from "../virtualModules";
import { isCssSideEffectImportPath } from "../../plugins/cssSideEffectImports";
import { createMatchPath } from "../../utils/tsconfig";
import { detectPackageManager } from "../../../cli/detectPackageManager";
import type { Context } from "../../context";
import { getLoaderForFile } from "../../utils/loaders";

Expand Down Expand Up @@ -85,36 +83,6 @@ export function serverBareModulesPlugin(ctx: Context): Plugin {
return undefined;
}

let packageName = getNpmPackageName(path);
let pkgManager = detectPackageManager() ?? "npm";

// Warn if we can't find an import for a package.
if (
!isNodeBuiltIn(packageName) &&
!/\bnode_modules\b/.test(importer) &&
// Silence spurious warnings when using Yarn PnP. Yarn PnP doesn’t use
// a `node_modules` folder to keep its dependencies, so the above check
// will always fail.
(pkgManager === "npm" ||
(pkgManager === "yarn" && process.versions.pnp == null))
) {
try {
require.resolve(path, { paths: [importer] });
} catch (error: unknown) {
ctx.logger.warn(`could not resolve "${path}"`, {
details: [
`You imported "${path}" in ${relative(
process.cwd(),
importer
)},`,
"but that package is not in your `node_modules`.",
"Did you forget to install it?",
],
key: path,
});
}
}

if (ctx.config.serverDependenciesToBundle === "all") {
return undefined;
}
Expand All @@ -138,17 +106,6 @@ export function serverBareModulesPlugin(ctx: Context): Plugin {
};
}

function isNodeBuiltIn(packageName: string) {
return builtinModules.includes(packageName);
}

function getNpmPackageName(id: string): string {
let split = id.split("/");
let packageName = split[0];
if (packageName.startsWith("@")) packageName += `/${split[1]}`;
return packageName;
}

function isBareModuleId(id: string): boolean {
return !id.startsWith("node:") && !id.startsWith(".") && !isAbsolute(id);
}

0 comments on commit cdd6927

Please sign in to comment.