Skip to content

feat: Add Option to Suppress Pre- and Post-Retrieve Events #854

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
Merged
merged 12 commits into from
Feb 17, 2023
Merged

feat: Add Option to Suppress Pre- and Post-Retrieve Events #854

merged 12 commits into from
Feb 17, 2023

Conversation

klewis-sfdc
Copy link
Contributor

What does this PR do?

  • Adds a property to ComponentSet RetrieveOptions to suppress the pre- and post- lifecycle events that are emitted when the ComponentSet's retrieve operation runs.

What issues does this PR fix or reference?

  • In the VSCE repo, work is underway to add source-tracking (STL) into the deploy* and retrieve* commands so that they can be used against scratch orgs (#4597).

  • In order to complete that work, two capabilities are needed:

    • VSCE needs to retrieve a file to a tmp location outside of the project
      • (Currently STL complains about the path used by os.tmpdir)
    • VSCE needs to be able to retrieve a file that has known conflicts after a deploy attempt that failed because of those conflicts (for line-by-line file diffing)

@W-12551198@

Functionality Before

  • No way to retrieve a ComponentSet without emitting pre- and post- retrieve events.
    • So STL is always being notified and reacting to the events

Functionality After

  • Using the suppressEvents option, a retrieve call can be made without emitting the events, therefore bypassing STL behaviors

Alternatives Considered:

  • Have STL not track files that are retrieved to a location outside of the project
    • Don't really want to make changes that try to tell STL what to track/not track
  • Turn off listening for the events in STL temporarily
    • VSCE uses long running processes, would have to remember to re-enable listener, could get into trouble in cases of exceptions

@klewis-sfdc klewis-sfdc requested review from a team as code owners February 15, 2023 18:32
@klewis-sfdc klewis-sfdc marked this pull request as draft February 15, 2023 20:35
klewis-sfdc
klewis-sfdc commented Feb 15, 2023
View reviewed changes
test/collections/componentSet.test.ts
preAndPostRetrieveEventCount = preAndPostRetrieveEventCount + 1;
}
});
expect(preAndPostRetrieveEventCount).to.equal(0);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only checking for the absence of Pre and Post retrieve events.

klewis-sfdc
klewis-sfdc commented Feb 15, 2023
View reviewed changes
src/client/types.ts
/**
* Specifies whether to suppress the <Pre|Post><Retrieve> events
*/
suppressEvents?: boolean;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only suppressing Pre and Post retrieve events. Other events are still fired (like apiVersionRetrieve).

klewis-sfdc
klewis-sfdc commented Feb 15, 2023
View reviewed changes
METADATA_SUPPORT.md
@@ -557,6 +557,7 @@ v58 introduces the following new types. Here's their current level of support
|Ai4mSettings|✅||
|CodeBuilderSettings|✅||
|DataWeaveResource|✅||
|DynamicFormsSettings|✅||
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this file auto-updated?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, from git hooks.

@klewis-sfdc klewis-sfdc marked this pull request as ready for review February 15, 2023 21:42
mshanemc
mshanemc approved these changes Feb 15, 2023
View reviewed changes
METADATA_SUPPORT.md
@@ -557,6 +557,7 @@ v58 introduces the following new types. Here's their current level of support
|Ai4mSettings|✅||
|CodeBuilderSettings|✅||
|DataWeaveResource|✅||
|DynamicFormsSettings|✅||
Copy link
Contributor

Choose a reason for hiding this comment

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

yes, from git hooks.

@mshanemc
Copy link
Contributor

@klewis-sfdc looks good to me. Can you have someone from VSCode QA and merge since y'all have a use case for it and we don't?

@klewis-sfdc
Copy link
Contributor Author

Yep, we can do QA and merge. Thanks, Shane.

gbockus-sf
gbockus-sf approved these changes Feb 17, 2023
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.

thanks @mshanemc & @klewis-sfdc great stuff

@gbockus-sf gbockus-sf merged commit 2eef7d5 into main Feb 17, 2023
@gbockus-sf gbockus-sf deleted the ken/suppress-retrieve-events branch February 17, 2023 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mshanemc mshanemc mshanemc approved these changes

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

@jeffb-sfdc jeffb-sfdc Awaiting requested review from jeffb-sfdc jeffb-sfdc was 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
@klewis-sfdc @mshanemc @gbockus-sf @svc-cli-bot