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(build): add copyPublicDir option #10550

Merged
merged 4 commits into from Oct 26, 2022
Merged

feat(build): add copyPublicDir option #10550

merged 4 commits into from Oct 26, 2022

Conversation

bluwy
Copy link
Member

@bluwy bluwy commented Oct 20, 2022

Description

This PR adds build.copyPublicDir: boolean option to control whether to copy the publicDir to outDir. Default true.

Additional context

Case

Vite default build copies files in publicDir to outDir. In common cases, this isn't useful for SSR builds as the client build already contains the publicDir files. To avoid unnecessary files, SvelteKit and Astro sets publicDir: false to turn it off.

Problem

However, turning it off disallows import img from '/favicon.png', which is possible with plugin-vue's asset transform:

<img src="/favicon.png" alt="" />

Which would error on SSR build as Vite can resolve /... imports from the publicDir. (withastro/astro#5063) (Extra base/publicDir discussion: #9276)

Solution

  1. New option like this PR
  2. Don't copy publicDir in SSR builds by default (could be breaking)
  3. Disable imports in publicDir as documented (Though the discussion above say otherwise Allow importing public files for base support #9276)

What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@bluwy bluwy added feat: ssr YAO Yet another option... feat: build labels Oct 20, 2022
@sapphi-red
Copy link
Member

I think this will fix #10082 (comment), too.
My current opinion is to go with this PR for Vite 3 and later go with 3. But I didn't take time to consider about this deeper so I might change my mind.

@bluwy
Copy link
Member Author

bluwy commented Oct 21, 2022

I think no3 is a bit of a special case that probably needs discussion too, but yeah with Vite 4 close, we could consider a nicer overall solution for this too.

@bluwy bluwy added the p2-to-be-discussed Enhancement under consideration (priority) label Oct 21, 2022
@patak-dev
Copy link
Member

We are looking to stabilize this feature. Request for feedback here:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: build feat: ssr p2-to-be-discussed Enhancement under consideration (priority) YAO Yet another option...
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants