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

Vite build fails, recursively creates public build directories when using assetsBuildDirectory #8023

Closed
justinwaite opened this issue Nov 16, 2023 · 17 comments

Comments

@justinwaite
Copy link
Contributor

justinwaite commented Nov 16, 2023

Reproduction

https://stackblitz.com/edit/remix-run-remix-fa3huu?file=package.json

System Info

System:
    OS: Linux 5.0 undefined
    CPU: (8) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 18.18.0 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 9.4.2 - /usr/local/bin/npm
    pnpm: 8.9.2 - /usr/local/bin/pnpm
  npmPackages:
    @remix-run/css-bundle: * => 2.2.0 
    @remix-run/dev: * => 2.2.0 
    @remix-run/eslint-config: * => 2.2.0 
    @remix-run/node: * => 2.2.0 
    @remix-run/react: * => 2.2.0 
    @remix-run/serve: * => 2.2.0 
    vite: ^4.5.0 => 4.5.0

Used Package Manager

npm

Expected Behavior

When running npm run build in an app setup with vite that has assetsBuildDirectory set to public/mybase/build, it is expected that the build would succeed and produce the public build artifacts at public/mybase/build.

Actual Behavior

If the public folder does not exist at all, then the build passes and creates public/mybase/build.

However, if the public directory exists, it tries to recursively create directories:

public
- mybase 
- - build
- - - mybase
- - - - build

Eventually, the build fails with the error:

error during build:
Error: ENAMETOOLONG: name too long, mkdir '/path/to/app/public/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase/build/mybase'
@justinwaite
Copy link
Contributor Author

It would be nice to support the vite base option that can be provided to vite.config.ts and respect that for the build outputs. This is something that we've wanted at my company since Remix launched, because we host many apps on the same origin, just different base paths.

@hi-ogawa
Copy link
Contributor

hi-ogawa commented Nov 16, 2023

Thanks for the reproduction!
I was also seeing some tricky behavior when customizing assetsBuildDirectory in my remix app (not well organized for others to see, but just for reference hi-ogawa/ytsub-v3#519).
What I ended up was to disable vite's automatic ./public directory copying behavior by passing publicDir: false when vite build.
I forked your repro to test this idea and it seems okay:
https://stackblitz.com/edit/remix-run-remix-qnj73f?file=vite.config.ts

export default defineConfig((env) => ({
  publicDir: env.command === 'build' ? false : undefined,
  plugins: [
    remix({
      assetsBuildDirectory: 'public/mybase/build',
      publicPath: '/mybase/build',
    }),
    tsconfigPaths(),
  ],
}));

This workaround might require extra script to copy some assets afterwards, but for the setup of your repro, ./public is assumed to be the root of the app, so I think it's fine. I confirmed favicon.ico is served for each case:

# after npm run dev
curl  http://localhost:5173/favicon.ico

# after npm run build && npm run start
curl  http://localhost:3000/favicon.ico

It would be nice to support the vite base option that can be provided to vite.config.ts and respect that for the build outputs. This is something that we've wanted at my company since Remix launched, because we host many apps on the same origin, just different base paths.

Regarding this, I think this might have fundamentally different technical challenge since this will not be only about vite's base option to control where vite recognize/emits assets, but also remix/react-router has to be aware of "base" path for the entire routing/navigation logic. I'm not sure if react-router already has such feature now, or any plan to do this.


Also FYI, remix plugin currently sets remix config's publicPath as vite's base option internally.

base: pluginConfig.publicPath,

So by default (which is vite base = remix publicPath = /build/), the generated assets look like this:

public/
- favicon.ico
- build/
  - favicon.ico  (<---- copied from vite `publicDir` to vite `base`)
  - manifest.json (vite manifest)
  - manifest-xxx.json (remix manifest)
  - assets/
    - ... all vite generated js chunks etc...

I was wondering if there is a way to achieve something similar without relying on base (for example, only by manipulating vite outDir?).

EDIT: I forgot about current remix's publicPath is also intended to be used with external domain, example from https://remix.run/docs/en/main/file-conventions/remix-config#publicpath

 publicPath: "https://static.example.com/assets/"

So, the usage of base seems actually correct for this scenario.

@pcattori pcattori added the vite label Nov 16, 2023
@hilja
Copy link

hilja commented Nov 16, 2023

Anyone else getting this warning?

(!) The public directory feature may not work correctly.
outDir /Users/bob/web/foo/public/build and publicDir /Users/bob/web/foo/public are not separate folders.

That's on the default settings.

@justinwaite
Copy link
Contributor Author

Anyone else getting this warning?


(!) The public directory feature may not work correctly.

outDir /Users/bob/web/foo/public/build and publicDir /Users/bob/web/foo/public are not separate folders.

That's on the default settings.

I did see these warnings intermittently throughout this process. I didn't nail down what it was though.

@hilja
Copy link

hilja commented Nov 16, 2023

Yeah same, I tried to finagle with the output settings but they felt quite unpredictable, because Remix is doing stuff under the hood and I'm not sure what, which @hi-ogawa just expanded on 👍

@nicksrandall
Copy link
Contributor

nicksrandall commented Nov 16, 2023

I'm seeing the same issue after migrating to Remix 2.3.0 and Vite v5. The ssr build is putting files in one directory too deep.

UPDATE: Actually, I'm re-reading this issue and realize my issue isn't with public dir but the build dir.

@markdalgleish
Copy link
Member

markdalgleish commented Nov 17, 2023

It seems like the way Remix has worked up to this point clashes with the way Vite typically works in terms of the public directory.

For now at least I think we'd want to maintain the same directory structure as the existing Remix compiler to make migration as painless as possible. To do this, we'll probably need to disable Vite's build.copyPublicDir option internally.

@markdalgleish
Copy link
Member

@nicksrandall Are you saying that you're running into a different problem? If so, could you open a separate issue?

@nicksrandall
Copy link
Contributor

@markdalgleish Sorry, the issue I'm facing is very similar and my guess is that a fix for one issue might fix the other. Anyway, here is the issue I created: #8038

@markdalgleish
Copy link
Member

I've opened a PR to fix this, but to confirm whether this works for you in the short term, try setting build.copyPublicDir to false in your Vite config:

export default defineConfig({
  build: {
    copyPublicDir: false
  },
  // etc.
});

@hi-ogawa
Copy link
Contributor

Nice! I wasn't aware of copyPublicDir option. This also helped my use case.

@justinwaite
Copy link
Contributor Author

I've opened a PR to fix this, but to confirm whether this works for you in the short term, try setting build.copyPublicDir to false in your Vite config:

export default defineConfig({
  build: {
    copyPublicDir: false
  },
  // etc.
});

I tried it briefly and it appears that it's working as intended. I didn't test too deeply or deploy anything yet, but at the very least the build passes and there are no recursive directories being created at build time.

@markdalgleish
Copy link
Member

Fixed by #8039.

Copy link
Contributor

🤖 Hello there,

We just published version 2.3.1-pre.0 which involves this issue. 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

Setting copyPublicDir: false solves the immediate problem, but unfortunately it turns out it also opens the door to bugs if your app code references any public assets, as raised in this comment on the PR linked above: #8039 (comment)

So, rather than fighting Vite's handling of the public directory and trying to maintain the way the existing Remix compiler works, we're looking at changing our directory structure to fit the Vite model: #8077

Copy link
Contributor

🤖 Hello there,

We just published version 2.3.1 which involves this issue. 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

The fix for this issue has been superseded by this PR: #8077

The updated fix also addresses the issue @hilja raised above: #8023 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants