Skip to content
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

Esbuild bundle the Python pool setup code #2886

Merged
merged 1 commit into from
Oct 10, 2024
Merged

Conversation

hoodmane
Copy link
Contributor

We want to execute a single standalone JavaScript file in the pool scope. It should work in a normal v8 isolate with no modifications. This adds the esbuild bundle step, but keeps everything else working the same.

@hoodmane hoodmane requested review from a team as code owners October 10, 2024 11:15
@@ -0,0 +1,103 @@
"""
Give a collection of js files a JsInfo provider so it can be used as a dependency for aspect_rules.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding a docstring!

@danlapid danlapid requested a review from fhanau October 10, 2024 11:18
@danlapid
Copy link
Collaborator

Tagging @fhanau for Bazel changes

WORKSPACE Outdated
@@ -274,6 +274,13 @@ http_archive(
url = "https://github.com/aspect-build/rules_ts/releases/download/v3.0.0/rules_ts-v3.0.0.tar.gz",
)

http_archive(
Copy link
Collaborator

@fhanau fhanau Oct 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Since we recently added update-deps.py for workerd, we should be using that for all new dependencies

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great to see a reduction in the number of differences between developing with workerd vs internally. How's it look now?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, LGTM! Didn't look closely at js_file but this should be good to go based on Dan's review

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well Dan wrote js_file.bzl =) I should probably add him as a coauthor on the commit.

@hoodmane hoodmane force-pushed the hoodmane/esbuild-for-pool branch 3 times, most recently from d33bdc5 to 6956521 Compare October 10, 2024 12:23
We want to execute a single standalone JavaScript file in the pool scope. It
should work in a normal v8 isolate with no modifications.
This adds the esbuild bundle step, but keeps everything else working the same.

Co-Authored-By: Dan Lapid <dan.lapid@gmail.com>
@hoodmane hoodmane force-pushed the hoodmane/esbuild-for-pool branch from 6956521 to cc93589 Compare October 10, 2024 12:52
@hoodmane hoodmane merged commit c9e01d3 into main Oct 10, 2024
12 of 13 checks passed
@hoodmane hoodmane deleted the hoodmane/esbuild-for-pool branch October 10, 2024 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants