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.

Sign up for GitHub

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

Jump to bottom

feat: rename lightning component bundles #3923

Merged
merged 28 commits into from
Mar 25, 2022
Merged

feat: rename lightning component bundles #3923

merged 28 commits into from
Mar 25, 2022

Conversation

floralan
Copy link
Contributor

@floralan floralan commented Mar 16, 2022 •
edited
Loading

What does this PR do?

Provide a new command: "SFDX: Rename Component" to rename LWC or Aura component. All files(js, html, css, js-meta.xml) for LWC component or all files(auradoc, cmp, cmp-meta.xml, css, design, svg, contoller, helper, renderer, js) for Aura component and the folder are automatically renamed with a new name that user provides.

What issues does this PR fix or reference?

@W-10288367@

Functionality Before

Right click "rename" to rename the files(at least 4 essential files for LWC and 9 essential files for Aura) and the folder one by one manually.

Functionality After

Right click with the new command: "SFDX: Rename Component", input the new component name, then all the files (that matches with the component name) and folder are renamed automatically.

@floralan floralan requested a review from a team as a code owner March 16, 2022 03:39
@floralan floralan requested a review from randi274 March 16, 2022 03:39
@floralan floralan changed the title feat: provide a new command to rename lightning component bundles feat: rename lightning component bundles Mar 16, 2022
@floralan floralan requested a review from jeffb-sfdc March 16, 2022 23:21
gbockus-sf
gbockus-sf requested changes Mar 18, 2022
View reviewed changes
Copy link
Contributor

@gbockus-sf gbockus-sf left a comment

Choose a reason for hiding this comment

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

Had some thoughts. Give em a read and let me know what you think.

gbockus-sf
gbockus-sf reviewed Mar 24, 2022
View reviewed changes
packages/salesforcedx-vscode-core/src/commands/forceRenameLightningComponent.ts
await renameComponent(this.sourceFsPath, newComponentName);
return true;
}
return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

swinging back around to the telemetry. We do get a ping about this command being called b/c of the LibraryCommandletExecutor extends...however we don't know if we actually do it as determined by this logic. It's fine to push that off for now as we need a plan, but I was just calling it out previously.

gbockus-sf
gbockus-sf reviewed Mar 24, 2022
View reviewed changes
packages/salesforcedx-vscode-core/src/commands/forceRenameLightningComponent.ts
return false;
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I still think everything below here should probably be in a service/util class...but get have have existing patterns we should break away from.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gbockus-sf This can be Dev Choice problem that we can discuss within the team later, yes, I followed the existing pattern.

gbockus-sf
gbockus-sf approved these changes Mar 24, 2022
View reviewed changes
Copy link
Contributor

@gbockus-sf gbockus-sf left a comment

Choose a reason for hiding this comment

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

some nits to consider. Thanks for they async'ing

...esforcedx-vscode-core/test/vscode-integration/commands/forceRenameLightningComponent.test.ts Outdated Show resolved Hide resolved
...esforcedx-vscode-core/test/vscode-integration/commands/forceRenameLightningComponent.test.ts
type: 'CONTINUE',
data: {name: 'hero1'}
});
expect(renameStub.callCount).to.equal(5);
Copy link
Contributor

Choose a reason for hiding this comment

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

dumb q: why is the count 5 here? Might be worth a code comment to call out what things we expect to be renamed if we are going to verify the call arguments.

Copy link
Contributor Author

@floralan floralan Mar 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@gbockus-sf The previous test focus on the call arguments, this test is intended to make sure it only rename the files and folder that have same name with LWC component. Here in this lwc component directory, it has 6 files and only 4 files match the component name, so rename should be called 5 times (plus renaming the folder rename).

...esforcedx-vscode-core/test/vscode-integration/commands/forceRenameLightningComponent.test.ts Outdated Show resolved Hide resolved
...esforcedx-vscode-core/test/vscode-integration/commands/forceRenameLightningComponent.test.ts Show resolved Hide resolved
...esforcedx-vscode-core/test/vscode-integration/commands/forceRenameLightningComponent.test.ts Outdated Show resolved Hide resolved
@floralan floralan merged commit 1a892f0 into develop Mar 25, 2022
@floralan floralan deleted the fl/rename_lightning_component branch March 25, 2022 00:59
@floralan floralan mentioned this pull request Apr 4, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gbockus-sf gbockus-sf gbockus-sf approved these changes

@RitamAgrawal RitamAgrawal RitamAgrawal left review comments

@jeffb-sfdc jeffb-sfdc jeffb-sfdc approved these changes

@randi274 randi274 Awaiting requested review from randi274 randi274 is a code owner automatically assigned from forcedotcom/ide-experience

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@floralan @jeffb-sfdc @RitamAgrawal @gbockus-sf