-
Notifications
You must be signed in to change notification settings - Fork 9
feat: Setup TypeScript, migrate libs with tests, migrate basic commands #211
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
Open
sbosio
wants to merge
12
commits into
prerelease/v1.0.0
Choose a base branch
from
sbosio/setup-typescript-and-libs
base: prerelease/v1.0.0
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
k80bowman
requested changes
Jul 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a couple of small comments, but overall this is amazing. Thank you for doing all of this and adding the tests. This is a big improvement.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This is the third PR on a series of pull requests required to migrate the Heroku Connect CLI Plugin to
@oclif/core
and TypeScript.On our initial PRs: #205 and #210 we just setup the repository according to our production standards for open sourced repositories and fixed the existing Mocha tests to ensure a smooth migration without introducing regressions, at least to the extent of the things that were tested (which is far less than ideal).
This third PR was meant to provide the required acceptance criteria on the linked work item at the bottom: namely the TypeScript configuration setup, migration of existing shared lib functions and adding tests were required to ensure the correct behavior. That proved to be challenging because:
@heroku-cli/command
) but leveraged Axios.standard
for linting purposes instead of ESLint.With all of that, this is the work done to meet the acceptance criteria (and a little more, for reasons):
package.json
to add required dependencies, upgrade versions to remove all vulnerabilities and remove dependencies onstandard
(replaced by ESLint),unexpected
(in favor ofchai
) andco
(use native async/await). That's what the first three commits on this PR are about./src/lib/types.ts
. There are still some unknowns that will need further refinement based on context requested to the Heroku Connect engineers./lib/**/*.js
to their destinations under/src/lib/*.ts
. File names and function names were preserved, but each one of those functions required refactoring to:co
and use native async/await./src/lib/api.ts
still purposely untested, awaiting on the same context requested to Heroku Connect engineers. There's a whole series of commands (connect-event:*
) that are listed as commands for some pilot being conducted that doesn't seem to have been promoted to GA at any point and we might want to remove all that dead code, reducing the migration effort scope on a 40%.Testing
This is also a little messy. But you don't have to suffer that much as I did.
yarn && yarn build
.heroku apps:create my-heroku-connect-test-app
heroku addons:create heroku-postgresql -a my-heroku-connect-test-app
heroku addons:create herokuconnect -a my-heroku-connect-test-app
heroku plugins:install @heroku-cli/heroku-connect-plugin
heroku addons:open herokuconnect -a my-heroku-connect-test-app
) and connect your add-on to the Salesforce Org with the credentials you just set. Create also a read-only Mapping to some Salesforce Org object, I choseFoo__c
because it only has 1 record.connect:write-errors
andconnect:mapping:write-errors
) both with the current plugin implementation and with the migrated commands (using./bin/run ...
instead ofheroku ...
).Merging
This PR is meant to be merged on the long-lived branch
prerelease/v1.0.0
.Screenshots (if applicable)
A couple screenshots with the usage of the implemented commands
connect:write-errors
andconnect:mapping:write-errors
ran both with the current plugin implementation and the migrated ones:SOC2 Compliance
Gus Work Item: W-17567704 (Heroku internal)