fix: embed version at compile time #237
Merged
Conversation
This file contains 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
kwasniew
commented
Jan 15, 2025
| @@ -25,6 +29,10 @@ export default { | |||
| } | |||
| ], | |||
| plugins: [ | |||
| replace({ | |||
| '__VERSION__': version, | |||
| preventAssignment: true | |||
There was a problem hiding this comment.
this is required by the warning from the package. more info here: https://www.npmjs.com/package/@rollup/plugin-replace
kwasniew
commented
Jan 15, 2025
| @@ -1383,7 +1381,8 @@ test('Should add `x-unleash` headers', async () => { | |||
| /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; | |||
|
|
|||
| const expectedHeaders = { | |||
| 'x-unleash-sdk': `unleash-js@${packageJSON.version}`, | |||
| // will be replaced at build time with the actual version | |||
There was a problem hiding this comment.
tests are working on pre-build version so no replacement
kwasniew
commented
Jan 15, 2025
| @@ -0,0 +1 @@ | |||
| export const sdkVersion = `unleash-js@__VERSION__`; | |||
There was a problem hiding this comment.
centralizing this to avoid typos in multiple places
thomasheartman
approved these changes
Jan 16, 2025
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.
About the changes
Removes requiring version at runtime with require: 3615f15#diff-258035e5968f6bf645400d417f310218d7d9a9a10606a3c34e7f55db193f58f3R3
Instead we're using the rollup replace plugin to read the version at build time and replace
__VERSION__placeholder with the actual version number.As a result other packages depending on this one don't need to understand require calls and also we're not exposing the whole package.json file at runtime.
To verify my claim above you can build the package and check the build directory after this change vs before this change.
Important files
Discussion points