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: getRootDir behaviour for single directory causing wrong project root #7251

Closed
wants to merge 2 commits into from

Conversation

akaltar
Copy link

@akaltar akaltar commented Nov 4, 2021

Linked issue: #7145

↪️ Pull Request

The getProjectRoot command was returning a wrong root when passed in a folder value. This caused the wrong project root to be determined when running in watch mode. In some cases this caused the entire filesystem to be watched.

The reason this is a draft is that this change breaks about ~100 integration tests, I'm happy to look into fixing those as well, but I'd like to first get feedback and confirm that my change aligns with the intended behaviour of the getProjectRoot function.

I tested this on a repo that was not working due to this bug, and now the .proxyrc.js file was loaded correctly and the site could be loaded. In fact some extra config files (.babelrc) also got loaded thanks to this.

Even if this is the intended change it's probably a huge breaking change, so please advise on how should I proceed.

Thank you for the amazing work on this project, adding a new test and making this fix was a breeze.

💻 Examples

Loading .proxyrc.js from project didn't work and explicitly setting the path for it wasn't possible.

🚨 Test instructions

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

@height
Copy link

height bot commented Nov 4, 2021

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

@akaltar akaltar changed the title [fix] Add getRootDir test and fix behaviour for single directory Fixes #7145 [DRAFT] [fix] Add getRootDir test and fix behaviour for single directory Fixes #7145 Nov 4, 2021
@akaltar akaltar changed the title [DRAFT] [fix] Add getRootDir test and fix behaviour for single directory Fixes #7145 [DRAFT] fix: getRootDir behaviour for single directory causing wrong project root Nov 5, 2021
@akaltar akaltar changed the title [DRAFT] fix: getRootDir behaviour for single directory causing wrong project root fix: getRootDir behaviour for single directory causing wrong project root Dec 15, 2021
@mischnic
Copy link
Member

mischnic commented Jun 7, 2022

Superseded by #7537

@mischnic mischnic closed this Jun 7, 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.

None yet

2 participants