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

feature: Allow import of vscode-util from other modules. #4426

Merged
merged 54 commits into from
Sep 27, 2022

Conversation

gbockus-sf
Copy link
Contributor

@gbockus-sf gbockus-sf commented Sep 14, 2022

What does this PR do?

This is the collection of changes needed to enable having the ability to expose the vscode-utils module as a single module in order to enable us to have a single location for core v3 updates.

How to Review

Mark this down..this is the only time you'll hear this from me...do NOT review this PR. It's too big. In order to make reviewing the changes simpler I have broken this PR apart into the following:
#4427
#4428
#4429
#4430
#4431
#4432
#4433
(please review those PRs). Note those PRs will not build or run locally. If you would like to verify the change please use the branch associated with this PR gbockus/allow-utils-in-faux-4

Changes found in this PR:

  • Update vscode-utils package to use a standard webpack build
  • Alter how we import from vscode-utils moving to just using the top level import @salesforce/salesforcedx-utils-vscode
  • Update all references to utils-vscode imports to use the common module instead of @salesforce/salesforcedx-vscode-utils/src/out/a/b/c
  • Add a new package @salesforce/salesforcedx-utils for non-vscode related common code (required and used by debuggers)
    • required due to these modules having tests that imported previously from vscode-utils. The test runner for those is specific to the vscode-debugger-protocol and couldn’t be migrated to jest so we needed a non-vscode module where we could pull in dependancies.
  • Update unit tests for majority of packages to use jest
    • Note: this is not update the existing tests to follow unit testing/jest best practices. It was the minimal effort to start using jest instead of mocha.
    • I have a plan to address the unit tests in the new package by writing the unit tests from scratch using jest there to lay down patterns to be used for all future unit test writing.
  • Fix a ton of stub references in integration tests.
    • The move to make a single export location for vscode-utils resulted in a lot of stub race conditions due to any load form the vscode-utils package would result in a module scan of the package so we end up not having stubs where we expect when using the import * as thing from 'here' pattern.
  • Upgrade typescript to support the jest upgrade

How will this merge?

After getting approval on the associated PRs I will incorporate any feedback and pull it into this branch. This branch is currently building successfully. We as a team can test that it is behaving property (@RitamAgrawal and I have built and run it locally successfully)
I will touch base in slack and merge after I have sign off from the team.

Tickets to be created as follow on work from review

  • clean up packages/salesforcedx-apex-debugger/test/unit/adapter/apexDebug.test.ts
  • Update @salesforce/salesforcedx-apex-replay-debugger to include main in package.json for making imports sane.
  • Add cleanup ticket for moving not vscode specific code to utils (NotNullOrUndefined for example )
  • Add linting rule to ensure we only throw Errors.
  • Remove the "useUnknownInCatchVariables": false setting from the tsconfig
  • Clean up in packages/salesforcedx-vscode-apex-replay-debugger/src/breakpoints/checkpointService.ts. See https://github.com/forcedotcom/salesforcedx-vscode/pull/4433/files/90528257504272fc70c6eb041bcb6dad31b86003# for various refactoring in that class
  • Unit test that only fails locally: Channel Service › Should pipe stdout on successful command execution

klewis-sfdc and others added 30 commits August 18, 2022 14:52
@gbockus-sf gbockus-sf marked this pull request as ready for review September 21, 2022 19:07
@gbockus-sf gbockus-sf requested a review from a team as a code owner September 21, 2022 19:07
@gbockus-sf gbockus-sf merged commit 71f396b into develop Sep 27, 2022
@gbockus-sf gbockus-sf deleted the gbockus/allow-utils-in-faux-4 branch September 27, 2022 15:31
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

3 participants