-
Notifications
You must be signed in to change notification settings - Fork 519
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
feat(esbuild): Script to update esbuild to the latest available version #2492
feat(esbuild): Script to update esbuild to the latest available version #2492
Conversation
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
scripts/update-esbuild-versions.js
Outdated
console.log('""" Generated code; do not edit\nUpdate by running yarn update-esbuild-versions\n\nHelper macro for fetching esbuild versions for internal tests and examples in rules_nodejs\n"""\n'); | ||
console.log('load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")\n') | ||
|
||
const latestVersion = JSON.parse(await getUrlAsString('https://registry.npmjs.org/esbuild/latest')).version; |
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.
Perhaps move the JSON.parse
into the getUrlAsString
method and refactor it to be called fetch
? It's the only placed its used, so doing the parse outside seems like it's implying in other places it's needed as a string.
scripts/update-esbuild-versions.js
Outdated
}; | ||
|
||
async function main() { | ||
console.log('""" Generated code; do not edit\nUpdate by running yarn update-esbuild-versions\n\nHelper macro for fetching esbuild versions for internal tests and examples in rules_nodejs\n"""\n'); |
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.
Perhaps preference, I like to add all the strings to an array, then join that array a output the result, rather than making multiple calls to console.log
, eg:
const content = [];
content.push(''"""Generated code; do not edit');
...
content.join('\n');
scripts/update-esbuild-versions.js
Outdated
console.log(` "https://registry.npmjs.org/${PLATFORMS[platform]}/-/${PLATFORMS[platform]}-%s.tgz" % version,`); | ||
console.log(' ],'); | ||
console.log(' strip_prefix = "package",'); | ||
console.log(' build_file_content = """exports_files(["bin/esbuild"])""",'); |
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.
Check the original file, the Windows build_file_content
is slightly different (see the Buildkite output too)
Just noticed this after sending out #2494 -- I think this script should also update the |
This should also update the repo in the examples folder too https://github.com/bazelbuild/rules_nodejs/blob/stable/examples/esbuild/WORKSPACE#L28 |
cbc038c
to
585c540
Compare
… only ever used as JSON
@JesseTatasciore ping us if you need help to get this green |
…ase note for version 0.9.0
examples/esbuild/WORKSPACE
Outdated
@@ -25,12 +25,12 @@ http_archive( | |||
urls = ["https://github.com/bazelbuild/rules_nodejs/releases/download/3.2.2/rules_nodejs-3.2.2.tar.gz"], | |||
) | |||
|
|||
_ESBUILD_VERSION = "0.8.48" | |||
_ESBUILD_VERSION = "0.9.6" |
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.
As there are breaking changes in this version, I don't think we should bump to it. This diff is about adding the update script (change one thing at a time :) )
replaceFileContent('packages/esbuild/_README.md', fileReplacements); | ||
} | ||
|
||
main(); |
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.
Does this always update to latest, or is there a way to force it to a version?
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.
Updated to allow for a version to be specified when running the script. Can be run as:
node ./scripts/update-esbuild-versions.js 0.8.57
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information