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: update to verify path exists before chdir in configAggProvider #4517

Merged
merged 1 commit into from
Oct 26, 2022

Conversation

gbockus-sf
Copy link
Contributor

@gbockus-sf gbockus-sf commented Oct 26, 2022

What does this PR do?

Update the config agg provider to not attempt to chdir when the current working directory is undefined or empty string.

What issues does this PR fix or reference?

#4465

Functionality Before

Screen Shot 2022-10-26 at 11 36 10 AM

Functionality After

Successful creation of a new project from the empty project state.

@gbockus-sf gbockus-sf requested a review from a team as a code owner October 26, 2022 16:37
@@ -82,12 +82,14 @@ export class ConfigAggregatorProvider {

private ensureCurrentDirectoryInsideProject(path: string) {
const rootWorkspacePath = workspaceUtils.getRootWorkspacePath();
if (path !== rootWorkspacePath) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

So the issue here is when we have an empty project state the rootWorkspacePath is ''. If we attempt to call process.chdir('') it throws an error and bails. It seemed reasonable to not attempt the chdir is we don't have a valid path. Def a little paranoid that we'll end up with a global config agg when we don't want it.

Copy link
Contributor

@klewis-sfdc klewis-sfdc left a comment

Choose a reason for hiding this comment

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

Looks good. Confirmed via QE that

✅ SFDX: Create Project works when not already in a project directory
✅ Once the project is created, ConfigAggregator for the root project workspace has local config values present

@gbockus-sf gbockus-sf merged commit 5fc4fdf into develop Oct 26, 2022
@gbockus-sf gbockus-sf deleted the gbockus/FixesForRelease branch October 26, 2022 20:08
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