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] don't log warning if root is configured #5330

Merged
merged 2 commits into from
Jul 1, 2022
Merged

Conversation

benmccann
Copy link
Member

root is no longer set since #5303. This logs an unnecessary warning when using Storybook

@changeset-bot
Copy link

changeset-bot bot commented Jun 30, 2022

🦋 Changeset detected

Latest commit: 4b3c0dc

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

This PR includes changesets to release 1 package
Name Type
@sveltejs/kit 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

@Rich-Harris
Copy link
Member

Can you elaborate? As far as I can tell, setting root just makes everything break. What are the situations in which it makes sense to be configured?

@benmccann
Copy link
Member Author

Storybook sets root to the expected directory. SvelteKit doesn't check what the value of the setting is and just prints an error if it's configured at all eventhough there's no actual issue. And SvelteKit doesn't actually set the value so the error message is also incorrect in saying that SvelteKit is taking control of it.

@Rich-Harris
Copy link
Member

What is 'the expected directory'? I'm just very unclear on how it could work. It feels like SvelteKit probably should set the value to the project root and/or print an error if it's set to something else

@benmccann
Copy link
Member Author

Storybook sets it to the project root, which is what I meant by expected directory

@Rich-Harris
Copy link
Member

Ah. So I think what we should do is error if root is explicitly set to something other than the cwd

@benmccann
Copy link
Member Author

Ok. Updated the PR to do that

@Rich-Harris Rich-Harris merged commit 45e1601 into master Jul 1, 2022
@Rich-Harris Rich-Harris deleted the root-enforcement branch July 1, 2022 19:49
@github-actions github-actions bot mentioned this pull request Jul 1, 2022
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.

2 participants