-
Notifications
You must be signed in to change notification settings - Fork 33
Cami/features component analysis #116
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
base: master
Are you sure you want to change the base?
Conversation
| function noCredentialsProvided() { | ||
| return (!signer) instanceof StaticCredentials || (!signer) instanceof SharedCredentials; | ||
| function credentialsProvided() { | ||
| return signer instanceof StaticCredentials || signer instanceof SharedCredentials; |
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.
Fixed this because it was working as a false positive. I also updated it to be the positive condition for readability
| * Adds to the request query to use the component analysis feature. | ||
| * @return Returns <b>this</b> to accommodate method chaining. | ||
| */ | ||
| withFeatureComponentAnalysis() { |
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.
Here is the error I get for this:
node examples/us_street.mjs
file:///Users/mitchell/Smarty/smartystreets-javascript-sdk/examples/us_street.mjs:21
let client = clientBuilder.buildUsStreetApiClient();
^
TypeError: Cannot read properties of undefined (reading 'buildUsStreetApiClient')
at file:///Users/mitchell/Smarty/smartystreets-javascript-sdk/examples/us_street.mjs:21:28
at ModuleJob.run (node:internal/modules/esm/module_job:234:25)
at async ModuleLoader.import (node:internal/modules/esm/loader:473:24)
at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:122:5)
Node.js v20.18.1
| * @return Returns <b>this</b> to accommodate method chaining. | ||
| */ | ||
| withFeatureComponentAnalysis() { | ||
| return this.withCustomCommaSeperatedQuery("features", "component-analysis"); |
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.
Same error with this:
`node examples/us_street.mjs
file:///Users/mitchell/Smarty/smartystreets-javascript-sdk/examples/us_street.mjs:21
let client = clientBuilder.buildUsStreetApiClient();
^
TypeError: Cannot read properties of undefined (reading 'buildUsStreetApiClient')
at file:///Users/mitchell/Smarty/smartystreets-javascript-sdk/examples/us_street.mjs:21:28
at ModuleJob.run (node:internal/modules/esm/module_job:234:25)
at async ModuleLoader.import (node:internal/modules/esm/loader:473:24)
at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:122:5)
Node.js v20.18.1`
https://app.clickup.com/t/86b77kdaf
See the clickup task for examples of how this was done in the Go SDK. I also cross referenced the Java SDK to help make sure I was understand the code correctly.
To review you can make sure the tests are running successfully (which you can see here in the PR have passed). You can also run the example code and modify it to use the new methods. When you test the examples a few things need to happen
npm run buildormake build(they do the same thing) to compile the SDK/examples/us_street.mjsimport SmartySDK from "../dist/esm/index.mjs";so it references the current changes from the branch, instead of referencing the published one