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

feat(esbuild): add support for multiple entry points #2663

Merged
merged 1 commit into from
May 10, 2021

Conversation

jbedard
Copy link
Collaborator

@jbedard jbedard commented May 8, 2021

Very similar to the rollup one, including copying (almost identically) the _desugar_entry_point_names util. The main difference is that rollup allows each entry point to have its own output file specified (see "Chunks can be named by adding an = to the provided value:" under https://rollupjs.org/guide/en/#input), where in esbuild it seems that you specify a output filename pattern which all outputted files follow: https://esbuild.github.io/api/#entry-names

In the future we can potentially add support for that --entry-names param, but I guess that can always be done manually today with the arbitrary args.

Note that the output_dir is now defaulted to True in the macro if entry_points is used. I'm not 100% sure if that is correct or the best way to do it? Are code splitting and entry_points starting to overlap a bit too much?

@jbedard jbedard requested a review from mattem May 8, 2021 07:54
@google-cla google-cla bot added the cla: yes label May 8, 2021
Copy link
Collaborator

@mattem mattem left a comment

Choose a reason for hiding this comment

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

Note that the output_dir is now defaulted to True in the macro if entry_points is used

We can't predict all the output files for multiple entry_points?

@@ -308,10 +337,11 @@ def esbuild_macro(name, output_dir = False, **kwargs):
entry_point = Label("@build_bazel_rules_nodejs//packages/esbuild:launcher.js"),
)

if output_dir == True:
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will cause an error when output_dir is True as it will pass entry_points = False, where entry_points is expecting a label list, but will get a boolean.

Perhaps something like the following (None is falsy and the default on label_list is an empty list)

entry_points = kwargs.get("entry_points", None)
if output_dir == True or entry_points:
    esbuild(
            name = name,
            output_dir = True,
            launcher = _launcher,
            **kwargs
        )

It can then be removed from the macro's signature

Comment on lines 193 to 194
This is just a shortcut for the `entry_points` attribute with a single entry.
Either this attribute or `entry_point` must be specified, but not both.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is copy / paste from rollup, but nit, I don't really like the word "just" 😛

Suggested change
This is just a shortcut for the `entry_points` attribute with a single entry.
Either this attribute or `entry_point` must be specified, but not both.
This is a shortcut for the `entry_points` attribute with a single entry.
Specify either this attribute or `entry_point`, but not both.

@@ -7,6 +7,22 @@ load("@build_bazel_rules_nodejs//:providers.bzl", "JSEcmaScriptModuleInfo", "JSM
load("@build_bazel_rules_nodejs//internal/linker:link_node_modules.bzl", "MODULE_MAPPINGS_ASPECT_RESULTS_NAME", "module_mappings_aspect")
load(":helpers.bzl", "filter_files", "generate_path_mapping", "resolve_entry_point", "write_jsconfig_file")

def _desugar_entry_point_names(entry_point, entry_points):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps this could go in helpers.bzl?

@jbedard
Copy link
Collaborator Author

jbedard commented May 9, 2021

We can't predict all the output files for multiple entry_points?

We can predict the output file for each entry point like when there's a single entry point. But we can not predict the shared chunks created for the same reasons as when doing code splitting (it's a chunk-[hash].js file, or might be a different format/location based on the --entry-names arg).

@jbedard jbedard force-pushed the esbuild-multi-entry-point branch from 59a5e7f to f90ee1f Compare May 9, 2021 18:29
@jbedard jbedard requested a review from mattem May 9, 2021 19:38
@mattem mattem merged commit b4f322a into bazel-contrib:stable May 10, 2021
twheys pushed a commit to twheys/rules_nodejs that referenced this pull request Jan 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants