fix: Update useragent string with npm version #13903
Conversation
This reverts commit 5960e13.
ashika112
changed the title
packages/core/src/Platform/index.ts
Outdated
| @@ -11,7 +11,7 @@ import { getCustomUserAgent } from './customUserAgent'; | |||
| const BASE_USER_AGENT = `aws-amplify`; | |||
|
|
|||
| class PlatformBuilder { | |||
| userAgent = `${BASE_USER_AGENT}/${version}`; | |||
| userAgent = `${BASE_USER_AGENT}/${version.replace(/\+.*/, '')}`; | |||
There was a problem hiding this comment.
Will changing this cause any issues with our metrics? Will we need to query for multiple versions of the version string now?
There was a problem hiding this comment.
Confirmed with product it does not affect them
There was a problem hiding this comment.
As we discussed, this is going to leave the preid versions with the hash duplication. Probably non-impacting, but a bit ugly. If PM is bought in on this variation, its not a blocker for any technical reasons.
There was a problem hiding this comment.
@stocaaro confirmed this wont cause an issue there since the current change will still preserve the hash. Its a duplicated bit of hash that is removed. Etc,
if version is 6.5.1-multi-bucket.7bc5681.0+7bc5681 -> 6.5.1-multi-bucket.7bc5681.0, hash 7bc5681 is preserved :)
|
|
||
| jest.mock('../../src/Platform/customUserAgent'); | ||
| const expectedVersion = version.replace(/\+.*/, ''); |
There was a problem hiding this comment.
nit: For testing purposes this could be a some hardcoded strings
There was a problem hiding this comment.
i was going to do that but then we will have to mock the version in first place so for now kept the same logic
There was a problem hiding this comment.
You could push the version replacement into a function and then test that function with a variety of inputs/outputs.
|
|
||
| jest.mock('../../src/Platform/customUserAgent'); | ||
| const expectedVersion = version.replace(/\+.*/, ''); |
There was a problem hiding this comment.
You could push the version replacement into a function and then test that function with a variety of inputs/outputs.
packages/core/src/Platform/index.ts
Outdated
| @@ -11,7 +11,7 @@ import { getCustomUserAgent } from './customUserAgent'; | |||
| const BASE_USER_AGENT = `aws-amplify`; | |||
|
|
|||
| class PlatformBuilder { | |||
| userAgent = `${BASE_USER_AGENT}/${version}`; | |||
| userAgent = `${BASE_USER_AGENT}/${version.replace(/\+.*/, '')}`; | |||
There was a problem hiding this comment.
As we discussed, this is going to leave the preid versions with the hash duplication. Probably non-impacting, but a bit ugly. If PM is bought in on this variation, its not a blocker for any technical reasons.
Description of changes
+. Lerna adds this special character+to the version and npm strips the value of post the+[Ref] we will do the same thing since we care about version in NPM for our metrics.yarn.locksince some version were backtracked weirdly last time we updated itDescription of how you validated changes
Checklist
yarn testpassesChecklist for repo maintainers
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.