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

fix: SFDX:Rename Component now also rename the test file that has the same name as component under __tests__ folder for LWC components #4020

Merged
merged 2 commits into from
Apr 12, 2022
Merged

fix: SFDX:Rename Component now also rename the test file that has the same name as component under __tests__ folder for LWC components #4020

merged 2 commits into from
Apr 12, 2022

Conversation

floralan
Copy link
Contributor

@floralan floralan commented Apr 11, 2022 •
edited
Loading

What does this PR do?

Enable SFDX: Rename Component command to rename the test file under __tests__ folder for LWC components. Only the test file that has the same name as component will be renamed.

What issues does this PR fix or reference?

@W-10976932@

Functionality Before

SFDX: Rename Component does not rename test file under tests folder in LWC components.

Functionality After

SFDX: Rename Component now rename the test file in LWC components as well.

@floralan floralan requested a review from a team as a code owner April 11, 2022 20:33
@floralan floralan requested a review from klewis-sfdc April 11, 2022 20:33
@floralan floralan changed the title fix: SFDX:Rename Component now also rename the test file under __tests__ folder that has the same name as component fix: SFDX:Rename Component now also rename the test file that has the same name as component under __tests__ folder Apr 11, 2022
@floralan floralan changed the title fix: SFDX:Rename Component now also rename the test file that has the same name as component under __tests__ folder fix: SFDX:Rename Component now also rename the test file that has the same name as component under __tests__ folder for LWC components Apr 11, 2022
@floralan floralan requested a review from sbudhirajadoc April 11, 2022 20:34
gbockus-sf
gbockus-sf reviewed Apr 11, 2022
View reviewed changes
...esforcedx-vscode-core/test/vscode-integration/commands/forceRenameLightningComponent.test.ts
@@ -39,7 +41,7 @@ describe('Force Rename Lightning Component', () => {
readdirStub
.onFirstCall().resolves([])
.onSecondCall().resolves([])
.onThirdCall().resolves([itemsInHero[1]]);
.onThirdCall().resolves([itemsInHero[0]]);
Copy link
Contributor

Choose a reason for hiding this comment

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

did this change for any particular reason?

Copy link
Contributor

Choose a reason for hiding this comment

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

oh I see... moving the tests got it

gbockus-sf
gbockus-sf approved these changes Apr 11, 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.

good stuff thanks for the quick turn around 👍

@gbockus-sf
Copy link
Contributor

gbockus-sf commented Apr 12, 2022 •
edited
Loading

QE:
✅ Verified that file with the name .test.js is renamed correctly
✅ Verified that files that do not match the naming convention are present but not renamed.

Bugs:
Found a destructive flow where if you have a file already named the new component name it is over written without warning. For example if my component is name accountMap and I have a file called __tests__/accountMapOther.test.js and I execute the command to rename to accountMapOther the file is replace with the current __tests__/accountMap.test.js file. Seems like a corner case so I'll create a bug to track it. Probably also applies to non-test files.

@floralan
Copy link
Contributor Author

floralan commented Apr 12, 2022 •
edited
Loading

@gbockus-sf I think it is because that "isDuplicate" function only check for the existing LWC and Aura Components name aka the folder names. If we check every file name under LWC or Aura for duplicate, when the number of components is large, "rename component" command can be slow. But we can try to implement it and test for the execution time to see if it's acceptable.

@gbockus-sf
Copy link
Contributor

ause that "isDuplicate" function only check for the existing LWC and Aura Components name aka the folder names. If we check every file name under LWC or Aura for duplicate, when the number of components is large, "rename component" command can be slow. But we can try to implement it and test for the execution time to see if it's acceptable.

@floralan I don't think it's a blocker for this PR. We can discuss the issue and see if it makes sense to worry about it. It would be good to add your above comment to the issue I created.

@floralan
Copy link
Contributor Author

@gbockus-sf Sounds great! I'll get this PR merged and discuss the issue in the charter of the ticket.

@floralan floralan merged commit 2da1195 into develop Apr 12, 2022
@floralan floralan deleted the fl/rename_test_file branch April 12, 2022 20:06
@sbudhirajadoc sbudhirajadoc removed their request for review April 19, 2022 00:04
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

@klewis-sfdc klewis-sfdc Awaiting requested review from klewis-sfdc klewis-sfdc 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.
2 participants
@floralan @gbockus-sf