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

build: include private exports in fesms #23054

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 0 additions & 12 deletions packages/bazel/src/BUILD.bazel
Expand Up @@ -26,15 +26,3 @@ nodejs_binary(
data = ["modify_tsconfig.js"],
entry_point = "angular/packages/bazel/src/modify_tsconfig.js",
)

nodejs_binary(
name = "index_bundler",
data = [
# BEGIN-INTERNAL
# Only needed when compiling within the Angular repo.
# Users will get this dependency from node_modules.
"//packages/compiler-cli",
# END-INTERNAL
],
entry_point = "@angular/compiler-cli/src/metadata/bundle_index_main.js",
)
165 changes: 74 additions & 91 deletions packages/bazel/src/ng_module.bzl
Expand Up @@ -12,7 +12,6 @@ load(":rules_typescript.bzl",
"compile_ts",
"DEPS_ASPECTS",
"ts_providers_dict_to_struct",
"json_marshal",
)

def _basename_of(ctx, file):
Expand All @@ -23,6 +22,36 @@ def _basename_of(ctx, file):
ext_len = len(".html")
return file.short_path[len(ctx.label.package) + 1:-ext_len]

def _flat_module_out_file(ctx):
"""Provide a default for the flat_module_out_file attribute.

We cannot use the default="" parameter of ctx.attr because the value is calculated
from other attributes (name)

Args:
ctx: skylark rule execution context

Returns:
a basename used for the flat module out (no extension)
"""
if hasattr(ctx.attr, "flat_module_out_file") and ctx.attr.flat_module_out_file:
return ctx.attr.flat_module_out_file
return "%s_public_index" % ctx.label.name

def _should_produce_flat_module_outs(ctx):
"""Should we produce flat module outputs.

We only produce flat module outs when we expect the ng_module is meant to be published,
based on the presence of the module_name attribute.

Args:
ctx: skylark rule execution context

Returns:
true iff we should run the bundle_index_host to produce flat module metadata and bundle index
"""
return bool(ctx.attr.module_name)

# Calculate the expected output of the template compiler for every source in
# in the library. Most of these will be produced as empty files but it is
# unknown, without parsing, which will be empty.
Expand All @@ -31,6 +60,7 @@ def _expected_outs(ctx):
closure_js_files = []
declaration_files = []
summary_files = []
metadata_files = []

factory_basename_set = depset([_basename_of(ctx, src) for src in ctx.files.factories])

Expand Down Expand Up @@ -73,13 +103,26 @@ def _expected_outs(ctx):
declaration_files += [ctx.new_file(ctx.bin_dir, basename + ext) for ext in declarations]
summary_files += [ctx.new_file(ctx.bin_dir, basename + ext) for ext in summaries]

# We do this just when producing a flat module index for a publishable ng_module
if _should_produce_flat_module_outs(ctx):
flat_module_out = _flat_module_out_file(ctx)
devmode_js_files.append(ctx.actions.declare_file("%s.js" % flat_module_out))
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe it's me being spoiled by javascript but I find this "backwards/printf" interpolation style less readable than using +.

it would be cool if skylark added support for f-string interpolation (f'Hello {name}! This is {program}') https://www.programiz.com/python-programming/string-interpolation.

Can we put that on our bazel wishlist?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

file a feature request? I'm only tracking things that will block adoption.

closure_js_files.append(ctx.actions.declare_file("%s.closure.js" % flat_module_out))
bundle_index_typings = ctx.actions.declare_file("%s.d.ts" % flat_module_out)
declaration_files.append(bundle_index_typings)
metadata_files.append(ctx.actions.declare_file("%s.metadata.json" % flat_module_out))
else:
bundle_index_typings = None

i18n_messages_files = [ctx.new_file(ctx.genfiles_dir, ctx.label.name + "_ngc_messages.xmb")]

return struct(
closure_js = closure_js_files,
devmode_js = devmode_js_files,
declarations = declaration_files,
summaries = summary_files,
metadata = metadata_files,
bundle_index_typings = bundle_index_typings,
i18n_messages = i18n_messages_files,
)

Expand All @@ -92,21 +135,29 @@ def _ngc_tsconfig(ctx, files, srcs, **kwargs):
def _ngc_tsconfig_helper(ctx, files, srcs, enable_ivy, **kwargs):
outs = _expected_outs(ctx)
if "devmode_manifest" in kwargs:
expected_outs = outs.devmode_js + outs.declarations + outs.summaries
expected_outs = outs.devmode_js + outs.declarations + outs.summaries + outs.metadata
else:
expected_outs = outs.closure_js

angular_compiler_options = {
"enableResourceInlining": ctx.attr.inline_resources,
"generateCodeForLibraries": False,
"allowEmptyCodegenFiles": True,
"enableSummariesForJit": True,
"enableIvy": enable_ivy,
"fullTemplateTypeCheck": ctx.attr.type_check,
# FIXME: wrong place to de-dupe
"expectedOut": depset([o.path for o in expected_outs]).to_list()
}

if _should_produce_flat_module_outs(ctx):
angular_compiler_options["flatModuleId"] = ctx.attr.module_name
angular_compiler_options["flatModuleOutFile"] = _flat_module_out_file(ctx)
angular_compiler_options["flatModulePrivateSymbolPrefix"] = "_".join(
[ctx.workspace_name] + ctx.label.package.split("/") + [ctx.label.name, ""])

return dict(tsc_wrapped_tsconfig(ctx, files, srcs, **kwargs), **{
"angularCompilerOptions": {
"enableResourceInlining": ctx.attr.inline_resources,
"generateCodeForLibraries": False,
"allowEmptyCodegenFiles": True,
"enableSummariesForJit": True,
"enableIvy": enable_ivy,
"fullTemplateTypeCheck": ctx.attr.type_check,
# FIXME: wrong place to de-dupe
"expectedOut": depset([o.path for o in expected_outs]).to_list()
}
"angularCompilerOptions": angular_compiler_options
})

def _collect_summaries_aspect_impl(target, ctx):
Expand Down Expand Up @@ -243,7 +294,7 @@ def _prodmode_compile_action(ctx, inputs, outputs, tsconfig_file, node_opts):

def _devmode_compile_action(ctx, inputs, outputs, tsconfig_file, node_opts):
outs = _expected_outs(ctx)
compile_action_outputs = outputs + outs.devmode_js + outs.declarations + outs.summaries
compile_action_outputs = outputs + outs.devmode_js + outs.declarations + outs.summaries + outs.metadata
_compile_action(ctx, inputs, compile_action_outputs, None, tsconfig_file, node_opts)

def _ts_expected_outs(ctx, label):
Expand All @@ -252,71 +303,6 @@ def _ts_expected_outs(ctx, label):
_ignored = [label]
return _expected_outs(ctx)

def _write_bundle_index(ctx):
""" Provide an action that runs the bundle_index_main in ngc.

The action will only be executed when bundling with ng_package with a dep[] on this ng_module.

This creates:
- "flat module" metadata files
- "bundle index" js files with private symbol re-export, and their accompanying .d.ts files

Args:
ctx: the skylark rule execution context

Returns:
A struct indicating where the files were produced, which is passed through an ng_package rule to packager.ts
"""
# Provide a default for the flat_module_out_file attribute.
# We cannot use the default="" parameter of ctx.attr because the value is calculated
# from other attributes (name)
flat_module_out_file = ctx.attr.flat_module_out_file if ctx.attr.flat_module_out_file else "%s_public_index" % ctx.label.name

tsconfig_file = ctx.actions.declare_file("%s.tsconfig.json" % flat_module_out_file)
metadata_file = ctx.actions.declare_file("%s.metadata.json" % flat_module_out_file)
typings_file = ctx.actions.declare_file("%s.d.ts" % flat_module_out_file)
index_file = ctx.actions.declare_file("%s.js" % flat_module_out_file)

tsconfig = dict(tsc_wrapped_tsconfig(ctx, ctx.files.srcs, ctx.files.srcs), **{
"angularCompilerOptions": {
"flatModuleOutFile": flat_module_out_file,
"flatModulePrivateSymbolPrefix": "_".join([ctx.workspace_name] + ctx.label.package.split("/") + [ctx.label.name, ""]),
},
})
if not ctx.attr.module_name:
fail("""Internal error: bundle index files are only produced for ng_module targets with a module_name.
Please file a bug at https://github.com/angular/angular/issues""")
tsconfig["angularCompilerOptions"]["flatModuleId"] = ctx.attr.module_name

entry_point = ctx.attr.entry_point if ctx.attr.entry_point else "index.ts"
# createBundleIndexHost in bundle_index_host.ts will throw if the "files" has more than one entry.
# We don't want to fail() here, however, because not all ng_module's will have the bundle index written.
# So we make the assumption that the index.ts file in the highest parent directory is the entry point.
index = None

for f in tsconfig["files"]:
if f.endswith("/" + entry_point):
if not index or len(f) < len(index):
index = f

if index:
tsconfig["files"] = [index]

ctx.actions.write(tsconfig_file, json_marshal(tsconfig))

ctx.action(
progress_message = "Producing metadata for bundle %s" % ctx.label.name,
executable = ctx.executable._index_bundler,
inputs = ctx.files.srcs + [tsconfig_file],
outputs = [metadata_file, typings_file, index_file],
arguments = ["-p", tsconfig_file.path],
)
return struct(
module_name = ctx.attr.module_name,
metadata_file = metadata_file,
typings_file = typings_file,
index_file = index_file)

def ng_module_impl(ctx, ts_compile_actions, ivy = False):
"""Implementation function for the ng_module rule.

Expand All @@ -343,18 +329,20 @@ def ng_module_impl(ctx, ts_compile_actions, ivy = False):

outs = _expected_outs(ctx)
providers["angular"] = {
"summaries": _expected_outs(ctx).summaries
"summaries": outs.summaries
}
providers["ngc_messages"] = outs.i18n_messages

# Only produces the flattened "index bundle" metadata when requested by some other rule
# and only under Bazel
if hasattr(ctx.executable, "_index_bundler") and ctx.attr.module_name:
bundle_index_metadata = [_write_bundle_index(ctx)]
else:
bundle_index_metadata = []
if _should_produce_flat_module_outs(ctx):
if len(outs.metadata) > 1:
fail("expecting exactly one metadata output for " + str(ctx.label))

providers["angular"]["flat_module_metadata"] = depset(bundle_index_metadata)
providers["angular"]["flat_module_metadata"] = struct(
module_name = ctx.attr.module_name,
metadata_file = outs.metadata[0],
typings_file = outs.bundle_index_typings,
flat_module_out_file = _flat_module_out_file(ctx),
)

return providers

Expand Down Expand Up @@ -422,11 +410,6 @@ NG_MODULE_RULE_ATTRS = dict(dict(COMMON_ATTRIBUTES, **NG_MODULE_ATTRIBUTES), **{
# See the flatModuleOutFile documentation in
# https://github.com/angular/angular/blob/master/packages/compiler-cli/src/transformers/api.ts
"flat_module_out_file": attr.string(),

"_index_bundler": attr.label(
executable = True,
cfg = "host",
default = Label("//packages/bazel/src:index_bundler")),
})

ng_module = rule(
Expand Down
29 changes: 16 additions & 13 deletions packages/bazel/src/ng_package/ng_package.bzl
Expand Up @@ -122,28 +122,31 @@ def _ng_package_impl(ctx):
# - ng_module rules in the deps (they have an "angular" provider)
# - in this package or a subpackage
# - those that have a module_name attribute (they produce flat module metadata)
entry_points = []
flat_module_metadata = []
for dep in ctx.attr.deps:
if dep.label.package.startswith(ctx.label.package):
entry_points.append(dep.label.package[len(ctx.label.package) + 1:])
if hasattr(dep, "angular") and dep.angular.flat_module_metadata:
flat_module_metadata.append(dep.angular.flat_module_metadata)
deps_in_package = [d for d in ctx.attr.deps if d.label.package.startswith(ctx.label.package)]
for dep in deps_in_package:
# Intentionally evaluates to empty string for the main entry point
entry_point = dep.label.package[len(ctx.label.package) + 1:]
if hasattr(dep, "angular") and hasattr(dep.angular, "flat_module_metadata"):
flat_module_metadata.append(dep.angular.flat_module_metadata)
flat_module_out_file = dep.angular.flat_module_metadata.flat_module_out_file + ".js"
else:
# fallback to a reasonable default
flat_module_out_file = "index.js"

for entry_point in entry_points:
es2015_entry_point = "/".join([p for p in [
ctx.bin_dir.path,
ctx.label.package,
ctx.label.name + ".es6",
ctx.label.package,
entry_point,
"index.js",
flat_module_out_file,
] if p])

es5_entry_point = "/".join([p for p in [
ctx.label.package,
entry_point,
"index.js",
flat_module_out_file,
] if p])

if entry_point:
Expand Down Expand Up @@ -192,10 +195,10 @@ def _ng_package_impl(ctx):
# Marshal the metadata into a JSON string so we can parse the data structure
# in the TypeScript program easily.
metadata_arg = {}
for m in depset(transitive = flat_module_metadata).to_list():
packager_inputs.extend([m.index_file, m.typings_file, m.metadata_file])
for m in flat_module_metadata:
packager_inputs.extend([m.metadata_file])
metadata_arg[m.module_name] = {
"index": m.index_file.path,
"index": m.typings_file.path.replace(".d.ts", ".js"),
"typings": m.typings_file.path,
"metadata": m.metadata_file.path,
}
Expand Down Expand Up @@ -226,7 +229,7 @@ def _ng_package_impl(ctx):
packager_args.add("")

ctx.actions.run(
progress_message = "Angular Packaging: building npm package for %s" % ctx.label.name,
progress_message = "Angular Packaging: building npm package %s" % str(ctx.label),
mnemonic = "AngularPackage",
inputs = packager_inputs,
outputs = [npm_package_directory],
Expand Down
5 changes: 0 additions & 5 deletions packages/bazel/src/ng_package/packager.ts
Expand Up @@ -165,16 +165,11 @@ function main(args: string[]): number {
// of the index JS file, which we need to amend the package.json.
Object.keys(modulesManifest).forEach(moduleName => {
const moduleFiles = modulesManifest[moduleName];
const indexContent = fs.readFileSync(moduleFiles['index'], 'utf-8');
const relative = path.relative(binDir, moduleFiles['index']);

moduleFiles['esm5_index'] = path.join(binDir, 'esm5', relative);
moduleFiles['esm2015_index'] = path.join(binDir, 'esm2015', relative);

writeFileFromInputPath(moduleFiles['esm5_index'], indexContent);
writeFileFromInputPath(moduleFiles['esm2015_index'], indexContent);

copyFileFromInputPath(moduleFiles['typings']);
copyFileFromInputPath(moduleFiles['metadata']);
});

Expand Down
14 changes: 12 additions & 2 deletions packages/bazel/src/ngc-wrapped/index.ts
Expand Up @@ -180,8 +180,18 @@ export function compile({allowNonHermeticReads, allDepsCompiledWithBazel = true,
return origBazelHostFileExist.call(bazelHost, fileName);
};
const origBazelHostShouldNameModule = bazelHost.shouldNameModule.bind(bazelHost);
bazelHost.shouldNameModule = (fileName: string) =>
origBazelHostShouldNameModule(fileName) || NGC_GEN_FILES.test(fileName);
bazelHost.shouldNameModule = (fileName: string) => {
// The bundle index file is synthesized in bundle_index_host so it's not in the
// compilationTargetSrc.
// However we still want to give it an AMD module name for devmode.
// We can't easily tell which file is the synthetic one, so we build up the path we expect
// it to have
// and compare against that.
if (fileName ===
path.join(compilerOpts.baseUrl, bazelOpts.package, compilerOpts.flatModuleOutFile + '.ts'))
return true;
return origBazelHostShouldNameModule(fileName) || NGC_GEN_FILES.test(fileName);
};

const ngHost = ng.createCompilerHost({options: compilerOpts, tsHost: bazelHost});

Expand Down
6 changes: 6 additions & 0 deletions packages/bazel/test/ng_package/core_package.spec.ts
Expand Up @@ -107,6 +107,9 @@ describe('@angular/core ng_package', () => {
expect(shx.cat('fesm2015/core.js'))
.toMatch(/@license Angular v\d+\.\d+\.\d+(?!-PLACEHOLDER)/);
});

it('should have been built from the generated bundle index',
() => { expect(shx.cat('fesm2015/core.js')).toMatch('export {.*makeParamDecorator'); });
});


Expand All @@ -127,6 +130,9 @@ describe('@angular/core ng_package', () => {
expect(shx.cat('fesm5/core.js')).not.toContain('function __extends');
expect(shx.cat('fesm5/core.js')).toMatch('import {.*__extends');
});

it('should have been built from the generated bundle index',
() => { expect(shx.cat('fesm5/core.js')).toMatch('export {.*makeParamDecorator'); });
});


Expand Down
Expand Up @@ -9,8 +9,8 @@ def _extract_flat_module_index(ctx):
files = []
for dep in ctx.attr.deps:
if hasattr(dep, "angular"):
for metadata in dep.angular.flat_module_metadata:
files.extend([metadata.metadata_file, metadata.typings_file])
metadata = dep.angular.flat_module_metadata
files.extend([metadata.metadata_file, metadata.typings_file])
return [DefaultInfo(files = depset(files))]

extract_flat_module_index = rule(
Expand Down