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

fix(dev): treat imported assets from node_modules as external #6813

Merged
merged 6 commits into from
Aug 8, 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
48 changes: 48 additions & 0 deletions integration/svg-in-node-modules-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { test, expect } from "@playwright/test";

import { PlaywrightFixture } from "./helpers/playwright-fixture";
import type { Fixture, AppFixture } from "./helpers/create-fixture";
import { createAppFixture, createFixture, js } from "./helpers/create-fixture";

let fixture: Fixture;
let appFixture: AppFixture;

test.beforeEach(async ({ context }) => {
await context.route(/_data/, async (route) => {
await new Promise((resolve) => setTimeout(resolve, 50));
route.continue();
});
});

test.beforeAll(async () => {
fixture = await createFixture({
files: {
"app/routes/_index.tsx": js`
import imgSrc from "getos/imgs/logo.svg";

export default function () {
return (
<div>
<img src={imgSrc} data-testid="example-svg" alt="example img"/>
</div>
)
}
`,
},
});

appFixture = await createAppFixture(fixture);
});

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

test("renders SVG images imported from node_modules", async ({ page }) => {
let app = new PlaywrightFixture(appFixture, page);
// You can test any request your app might get using `fixture`.
await app.goto("/");
expect(await page.getByTestId("example-svg").getAttribute("src")).toMatch(
/\/build\/_assets\/logo-.*\.svg/
);
});
20 changes: 18 additions & 2 deletions packages/remix-dev/compiler/server/plugins/bareImports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ 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";

/**
* A plugin responsible for resolving bare module ids based on server target.
Expand Down Expand Up @@ -59,8 +60,23 @@ export function serverBareModulesPlugin(ctx: Context): Plugin {
return undefined;
}

// Always bundle CSS files so we get immutable fingerprinted asset URLs.
if (path.endsWith(".css")) {
// Skip assets that are treated as files (.css, .svg, .png, etc.).
// Otherwise, esbuild would emit code that would attempt to require()
// or import these files --- which aren't JavaScript!
let loader;
try {
loader = getLoaderForFile(path);
} catch (e) {
if (
!(
e instanceof Error &&
e.message.startsWith("Cannot get loader for file")
)
) {
throw e;
}
}
if (loader === "file") {
return undefined;
}

Expand Down