add: config error if outDir is inside publicDir
#8152
Conversation
🦋 Changeset detectedLatest commit: a195f87 The changes in this PR will be included in the next version bump. 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 |
github-actions
bot
added
the
pkg: astro
|
Sorry, I merged without checking Nate's comment. @withastro/maintainers-docs please leave your feedback here and we will do a follow up PR |
There was a problem hiding this comment.
Thanks for the ping, @ematipico! Left a suggestion for you and @natemoo-re to look at, for your consideration!
| it('Error when outDir is placed within publicDir', async () => { | ||
| const configError = await validateConfig({ outDir: './public/dist' }, process.cwd()).catch((err) => err); | ||
| expect(configError instanceof z.ZodError).to.equal(true); | ||
| expect(configError.errors[0].message).to.equal('`outDir` must not be placed inside `publicDir` to prevent an infinite loop. \ |
There was a problem hiding this comment.
expect(configError.errors[0].message).to.equal(' The value of `outDir` must not point to a path in the `public/` folder to prevent an infinite loop. \
Something like this? Talking about putting "one configuration option inside another" isn't as clear as it could be. (At first, I was assuming this error meant that someone treated outDir as not a top-level root config..., i.e. the literal position of the config option within your .astro file.)
I think making the message explicitly refer to the value outDir can or cannot take is more helpful guidance? If "somewhere within the public folder" doesn't cover all instances given whatever might be set in publicDir, then the message could be something like:
The value of
outDirmust not point to a path within the folder set aspublicDirto prevent an infinite loop.
There was a problem hiding this comment.
The second one sounds good to me. I can make a new PR changing the message. Should I keep the last part: "Please adjust the directory configuration and try again"?
Changes
fixes #6904
I thought that creating a ZodError in schema would be better than throwing a new error so it works for both ssr and ssg and we just need to change one file. What do you think?
And is this message ok?
Testing
New test expecting an error message.
Docs
n/a