chore: update script library #939
Conversation
| const path = 'src/lib-prefix.ts' | ||
| if (!fs.existsSync(path)) { | ||
| // Supported in rollup 4, we're currently rollup 2 | ||
| // this.error(`File not found: ${path}`); |
There was a problem hiding this comment.
Minor, do we want to keep this logging comment?
There was a problem hiding this comment.
I'd like to keep it here so that when we migrate to rollup 4, we can just uncomment this line. wdyt?
| } | ||
|
|
||
| fs.writeFileSync(path, updatedContent, 'utf-8'); | ||
| // this.info(`File updated: ${path}`); |
There was a problem hiding this comment.
the same as above #939 (comment)
There was a problem hiding this comment.
Nit: Should we also add the comment // Supported in rollup 4, we're currently rollup 2 here for clarity
| @@ -63,6 +97,7 @@ export const iife = { | |||
| sourcemap: true, | |||
| }, | |||
| plugins: [ | |||
| updateLibPrefixPlugin(), | |||
There was a problem hiding this comment.
I think another possible solution is using https://www.npmjs.com/package/rollup-plugin-inject-process-env, so we can dynamically inject environment variable here, then read in the build process.
Manipulating the library file solution LGTM too.
There was a problem hiding this comment.
Great solution! I'll make it as a backup if the plugin solution doesn't work well.
| } | ||
|
|
||
|
|
||
| const updateLibPrefixPlugin = (isUndo) => { |
There was a problem hiding this comment.
Doesn't look like isUndo is used here
| } | ||
|
|
||
| fs.writeFileSync(path, updatedContent, 'utf-8'); | ||
| // this.info(`File updated: ${path}`); |
There was a problem hiding this comment.
Nit: Should we also add the comment // Supported in rollup 4, we're currently rollup 2 here for clarity
| @@ -54,6 +55,39 @@ export const umd = { | |||
| ], | |||
| }; | |||
|
|
|||
| const updateLibPrefix = (isUndo) => { | |||
There was a problem hiding this comment.
Nit: Should function name give a bit more context? Not sure if I'm just unfamiliar with rollup/context of the PR. isUndo makes a bit more sense with some context.
Summary
amplitude-ts-sdk-scriptin amplitude-min.js which is renamed asanalytics-browser-x.x.x-min.js.gzand uploaded to s3 atAmplitude-TypeScript/scripts/publish/upload-to-s3.js
Line 9 in cbb0afa
amplitude-tsamplitude-tsChecklist