From e736573a407a15617762d86c4a2ad4d2da5e977a Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Wed, 28 Mar 2018 18:15:36 -0700 Subject: [PATCH] fix(bazel): ng_package should include private exports in fesms --- packages/bazel/src/BUILD.bazel | 12 -- packages/bazel/src/ng_module.bzl | 165 ++++++++---------- packages/bazel/src/ng_package/ng_package.bzl | 29 +-- packages/bazel/src/ng_package/packager.ts | 5 - packages/bazel/src/ngc-wrapped/index.ts | 14 +- .../test/ng_package/core_package.spec.ts | 6 + .../ng_module/extract_flat_module_index.bzl | 4 +- .../src/metadata/bundle_index_host.ts | 29 ++- .../src/metadata/bundle_index_main.ts | 43 ----- packages/compiler-cli/tsconfig-build.json | 1 - 10 files changed, 133 insertions(+), 175 deletions(-) delete mode 100644 packages/compiler-cli/src/metadata/bundle_index_main.ts diff --git a/packages/bazel/src/BUILD.bazel b/packages/bazel/src/BUILD.bazel index 661afb4bdf284..fbe921dbecb8e 100644 --- a/packages/bazel/src/BUILD.bazel +++ b/packages/bazel/src/BUILD.bazel @@ -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", -) diff --git a/packages/bazel/src/ng_module.bzl b/packages/bazel/src/ng_module.bzl index 2e9cceb91018f..82c609dddf766 100644 --- a/packages/bazel/src/ng_module.bzl +++ b/packages/bazel/src/ng_module.bzl @@ -12,7 +12,6 @@ load(":rules_typescript.bzl", "compile_ts", "DEPS_ASPECTS", "ts_providers_dict_to_struct", - "json_marshal", ) def _basename_of(ctx, file): @@ -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. @@ -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]) @@ -73,6 +103,17 @@ 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)) + 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( @@ -80,6 +121,8 @@ def _expected_outs(ctx): 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, ) @@ -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): @@ -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): @@ -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. @@ -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 @@ -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( diff --git a/packages/bazel/src/ng_package/ng_package.bzl b/packages/bazel/src/ng_package/ng_package.bzl index 243045f3b0b68..e095582c287c7 100644 --- a/packages/bazel/src/ng_package/ng_package.bzl +++ b/packages/bazel/src/ng_package/ng_package.bzl @@ -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: @@ -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, } @@ -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], diff --git a/packages/bazel/src/ng_package/packager.ts b/packages/bazel/src/ng_package/packager.ts index aedd949632b00..649f79aa9f88f 100644 --- a/packages/bazel/src/ng_package/packager.ts +++ b/packages/bazel/src/ng_package/packager.ts @@ -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']); }); diff --git a/packages/bazel/src/ngc-wrapped/index.ts b/packages/bazel/src/ngc-wrapped/index.ts index b16a53b6a7d23..44f38eb276d65 100644 --- a/packages/bazel/src/ngc-wrapped/index.ts +++ b/packages/bazel/src/ngc-wrapped/index.ts @@ -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}); diff --git a/packages/bazel/test/ng_package/core_package.spec.ts b/packages/bazel/test/ng_package/core_package.spec.ts index d38102a466727..bbe523203d95f 100644 --- a/packages/bazel/test/ng_package/core_package.spec.ts +++ b/packages/bazel/test/ng_package/core_package.spec.ts @@ -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'); }); }); @@ -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'); }); }); diff --git a/packages/compiler-cli/integrationtest/bazel/ng_module/extract_flat_module_index.bzl b/packages/compiler-cli/integrationtest/bazel/ng_module/extract_flat_module_index.bzl index a7d6e3c0c319c..acbb631276fc5 100644 --- a/packages/compiler-cli/integrationtest/bazel/ng_module/extract_flat_module_index.bzl +++ b/packages/compiler-cli/integrationtest/bazel/ng_module/extract_flat_module_index.bzl @@ -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( diff --git a/packages/compiler-cli/src/metadata/bundle_index_host.ts b/packages/compiler-cli/src/metadata/bundle_index_host.ts index ea071ade88223..198c48fc487f8 100644 --- a/packages/compiler-cli/src/metadata/bundle_index_host.ts +++ b/packages/compiler-cli/src/metadata/bundle_index_host.ts @@ -37,7 +37,11 @@ function createSyntheticIndexHost( newHost.getSourceFile = (fileName: string, languageVersion: ts.ScriptTarget, onError?: (message: string) => void) => { if (path.normalize(fileName) == normalSyntheticIndexName) { - return ts.createSourceFile(fileName, indexContent, languageVersion, true); + const sf = ts.createSourceFile(fileName, indexContent, languageVersion, true); + if ((delegate as any).fileNameToModuleName) { + sf.moduleName = (delegate as any).fileNameToModuleName(fileName); + } + return sf; } return delegate.getSourceFile(fileName, languageVersion, onError); }; @@ -48,10 +52,10 @@ function createSyntheticIndexHost( sourceFiles: Readonly[]) => { delegate.writeFile(fileName, data, writeByteOrderMark, onError, sourceFiles); if (fileName.match(DTS) && sourceFiles && sourceFiles.length == 1 && - path.normalize(sourceFiles[0].fileName) == normalSyntheticIndexName) { + path.normalize(sourceFiles[0].fileName) === normalSyntheticIndexName) { // If we are writing the synthetic index, write the metadata along side. const metadataName = fileName.replace(DTS, '.metadata.json'); - fs.writeFileSync(metadataName, indexMetadata, {encoding: 'utf8'}); + delegate.writeFile(metadataName, indexMetadata, writeByteOrderMark, onError, []); } }; return newHost; @@ -61,7 +65,20 @@ export function createBundleIndexHost( ngOptions: CompilerOptions, rootFiles: ReadonlyArray, host: H): {host: H, indexName?: string, errors?: ts.Diagnostic[]} { const files = rootFiles.filter(f => !DTS.test(f)); - if (files.length != 1) { + let indexFile: string|undefined; + if (files.length === 1) { + indexFile = files[0]; + } else { + for (const f of files) { + // Assume the shortest file path called index.ts is the entry point + if (f.endsWith(path.sep + 'index.ts')) { + if (!indexFile || indexFile.length > f.length) { + indexFile = f; + } + } + } + } + if (!indexFile) { return { host, errors: [{ @@ -75,8 +92,8 @@ export function createBundleIndexHost( }] }; } - const file = files[0]; - const indexModule = file.replace(/\.ts$/, ''); + + const indexModule = indexFile.replace(/\.ts$/, ''); const bundler = new MetadataBundler( indexModule, ngOptions.flatModuleId, new CompilerHostAdapter(host), ngOptions.flatModulePrivateSymbolPrefix); diff --git a/packages/compiler-cli/src/metadata/bundle_index_main.ts b/packages/compiler-cli/src/metadata/bundle_index_main.ts deleted file mode 100644 index b2ed39b4163f9..0000000000000 --- a/packages/compiler-cli/src/metadata/bundle_index_main.ts +++ /dev/null @@ -1,43 +0,0 @@ -#!/usr/bin/env node -/** - * @license - * Copyright Google Inc. All Rights Reserved. - * - * Use of this source code is governed by an MIT-style license that can be - * found in the LICENSE file at https://angular.io/license - */ - -// Must be imported first, because Angular decorators throw on load. -import 'reflect-metadata'; - -import * as ts from 'typescript'; -import * as path from 'path'; -import {readCommandLineAndConfiguration} from '../main'; -import {createBundleIndexHost} from './bundle_index_host'; -import * as ng from '../transformers/entry_points'; - -export function main(args: string[], consoleError: (s: string) => void = console.error): number { - const {options, rootNames} = readCommandLineAndConfiguration(args); - const host = ng.createCompilerHost({options}); - const {host: bundleHost, indexName, errors} = createBundleIndexHost(options, rootNames, host); - if (!indexName) { - console.error('Did not find an index.ts in the top-level of the package.'); - return 1; - } - // The index file is synthetic, so we have to add it to the program after parsing the tsconfig - rootNames.push(indexName); - const program = ts.createProgram(rootNames, options, bundleHost); - const indexSourceFile = program.getSourceFile(indexName); - if (!indexSourceFile) { - console.error(`${indexSourceFile} is not in the program. Please file a bug.`); - return 1; - } - program.emit(indexSourceFile); - return 0; -} - -// CLI entry point -if (require.main === module) { - const args = process.argv.slice(2); - process.exitCode = main(args); -} diff --git a/packages/compiler-cli/tsconfig-build.json b/packages/compiler-cli/tsconfig-build.json index 1bc16e72e0458..6c4bddb6bfeb4 100644 --- a/packages/compiler-cli/tsconfig-build.json +++ b/packages/compiler-cli/tsconfig-build.json @@ -27,7 +27,6 @@ "index.ts", "ngtools2.ts", "src/main.ts", - "src/metadata/bundle_index_main.ts", "src/extract_i18n.ts", "src/language_services.ts", "../../node_modules/@types/node/index.d.ts",