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(vite): disable redundant copyPublicDir option #8039

Merged
merged 2 commits into from
Nov 17, 2023

Conversation

markdalgleish
Copy link
Member

@markdalgleish markdalgleish commented Nov 17, 2023

Fixes #8023

From the changeset:

Fix redundant copying of assets from public directory in Vite build. This ensures that static assets aren't duplicated in the server build directory. This also fixes an issue where the build would break if assetsBuildDirectory was deeply nested within the public directory.

Copy link

changeset-bot bot commented Nov 17, 2023

🦋 Changeset detected

Latest commit: c8d9a50

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 16 packages
Name Type
@remix-run/dev Patch
create-remix Patch
remix Patch
@remix-run/architect Patch
@remix-run/cloudflare Patch
@remix-run/cloudflare-pages Patch
@remix-run/cloudflare-workers Patch
@remix-run/css-bundle Patch
@remix-run/deno Patch
@remix-run/eslint-config Patch
@remix-run/express Patch
@remix-run/node Patch
@remix-run/react Patch
@remix-run/serve Patch
@remix-run/server-runtime Patch
@remix-run/testing Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@markdalgleish markdalgleish merged commit f7062e4 into dev Nov 17, 2023
9 checks passed
@markdalgleish markdalgleish deleted the markdalgleish/vite-disable-copy-public-dir branch November 17, 2023 22:23
@github-actions github-actions bot added the awaiting release This issue has been fixed and will be released soon label Nov 17, 2023
@gustavopch
Copy link
Contributor

gustavopch commented Nov 18, 2023

I was loading custom fonts in my app/global.css like this:

@font-face {
  font-display: swap;
  font-family: 'Poppins';
  font-style: normal;
  font-weight: 500;
  src: url('/fonts/poppins-v19-latin-500.woff2') format('woff2');
}

The fonts were located in my project in the public/fonts folder, so with copyPublicDir: true, they would be copied to public/build/fonts. Turns out with copyPublicDir: false they fail to load in production because the output CSS tries to load them with a build/ prefix url('/build/fonts/poppins-v19-latin-500.woff2') and now they're not copied to there anymore. Note that in development they're still loaded from the right place, without this build/ prefix.

So either I'm loading the fonts wrong or there's another issue that's now surfaced with copyPublicDir: false.

As a workaround, I'm moving the fonts from public/fonts to app/fonts and using a relative import like url('./fonts/poppins-v19-latin-500.woff2'). Maybe that's just how it's supposed to be done anyway. If so, then it's worth mentioning this change in the migration doc.

@markdalgleish
Copy link
Member Author

markdalgleish commented Nov 19, 2023

@gustavopch Your issue is an interesting one to me. I didn't realise that Vite rewrites references to files in your public directory when present in the CSS. Your fix of moving the file to the app directory is what I would have recommended, but I think this is highlighting a disconnect between the way the Remix compiler worked and the way Vite works when it comes to directory structure and public files. I might need to dig into this further.

Copy link
Contributor

🤖 Hello there,

We just published version 2.3.1-pre.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@markdalgleish
Copy link
Member Author

@gustavopch I've opened a PR that will fix your issue: #8077. This means that the directory structure needs to change when using Vite so it's not a small change, but I'm glad this got picked up relatively early.

Copy link
Contributor

🤖 Hello there,

We just published version 2.3.1 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions github-actions bot removed the awaiting release This issue has been fixed and will be released soon label Nov 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants