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: warn when publicDir and outDir are nested #13742

Merged
merged 6 commits into from
Jul 17, 2023

Conversation

patak-dev
Copy link
Member

Description

There have been a few reports of people having issues debugging after setting an outDir and publicDir that are nested. I think Vite should disable the public dir feature and warn the user in this case.

Example:

export default {
  build: {
    outDir: 'public/assets/dist',
  },
};

See discussion at Discord from @fvsch and a previous issue #3173

@stackblitz
Copy link

stackblitz bot commented Jul 7, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@12dgyyytewz2235
Copy link

Hi

Copy link
Member

@ArnaudBarre ArnaudBarre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any case where it's useful to have this?
It could create a difference between dev & preview

@patak-dev
Copy link
Member Author

If cleanOutDir is false, the size of dist would spiral out of control. If we want to avoid the diff, we could emit the warning and copy things anyways.

@ArnaudBarre
Copy link
Member

My point is that probably this is a misconfiguration of user that don't understand how the public folder works and we should throw instead if there is no good usecase for this.
And if they are very specific, user could still disable copyPublicDir and do copy("dist", "public/dist") on a post build step

bluwy
bluwy previously approved these changes Jul 17, 2023
Copy link
Member

@bluwy bluwy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'm ok with a warning as a start. I could see some setups wanting the outDir to be in publicDir intentionally, and is fine to suppress the warning.

@patak-dev
Copy link
Member Author

I think I'm ok with a warning as a start. I could see some setups wanting the outDir to be in publicDir intentionally, and is fine to suppress the warning.

Currently the PR will bail out of copying the publicDir content to outDir, and not only do a warning. Do you think we should only issue the warning? Probably not a bad idea for a patch.

Do you have any example of when outDir should be in publicDir? It seems to me that we should have thrown from the start in this case and it isn't something we should support. If we keep copying and only do a warning now, I would still lean to start throwing in Vite 5

@bluwy
Copy link
Member

bluwy commented Jul 17, 2023

Ah ok I didn't realize we support publicDir: false, which I thought if they want to achieve that they can suppress the warning instead. In that case I think we can follow your idea to only warn + keep copying, then in Vite 5 we can error out instead (or skip copying).

Do you have any example of when outDir should be in publicDir?

In the past when Svelte's main tooling is Rollup, the convention is to build into the public directory, that way you deploy your project through the public directory instead. It's probably rare to have this setup with Vite now but I remember seeing some setups like this before on Discord/Discussions.

@ArnaudBarre
Copy link
Member

I don't see how copy public inside public dir is a wanted setup. I don't think going for throw now will be an issue, but because I think the this very unlikely given the disk issue with multiple build, it's fine for me now or in Vite 5

@patak-dev
Copy link
Member Author

Ok, let's start emitting the warning so we can start throwing in Vite 5. I did that here:

@patak-dev patak-dev requested a review from bluwy July 17, 2023 10:07
@patak-dev patak-dev merged commit 4eb3154 into main Jul 17, 2023
@patak-dev patak-dev deleted the feat/warn-when-public-and-outdir-are-nested branch July 17, 2023 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants