Skip to content

Commit

Permalink
fix(bazel): ng_package rule should update "package.json" of ts_librar…
Browse files Browse the repository at this point in the history
…y targets (#36944)

In the past we added support for `ts_library` to `ng_package`. For those
targets we never can determine the "index" file. Unlike `ng_module`,
there is no provider data for flat module bundles, so the `ng_package`
rule assumes that the index file is simply called `index`.

This works as expected, but we also added logic in the past that doesn't
allow `ng_package` to add format properties (e.g. `main`, `module`) to a
`package.json` if a package json is handwritten for such a `ts_library` target.

This has been done that way because we assumed that such `package.json` files
might want to set format properties explicitly to different paths due to a
faulty "index" guess.

We want to change this behavior as most of the time a `package.json`
file already exists with just the module name. In those cases, the
packager should still set the format properties. We should only warn
and skip automatic insertion of the format properties if such a
`package.json` explicitly sets format properties.

PR Close #36944
  • Loading branch information
devversion authored and alxhub committed May 6, 2020
1 parent d578ab8 commit d5293d2
Showing 1 changed file with 30 additions and 5 deletions.
35 changes: 30 additions & 5 deletions packages/bazel/src/ng_package/packager.ts
Expand Up @@ -102,6 +102,13 @@ function main(args: string[]): number {
const modulesManifest = JSON.parse(modulesManifestArg);
const dtsBundles: string[] = dtsBundleArg.split(',').filter(s => !!s);

/**
* List of known `package.json` fields which provide information about
* supported package formats and their associated entry paths.
*/
const knownFormatPackageJsonFields =
['main', 'fesm2015', 'esm2015', 'typings', 'module', 'es2015'];

if (readmeMd) {
copyFile(readmeMd, out);
}
Expand Down Expand Up @@ -324,11 +331,10 @@ function main(args: string[]): number {
const packageName = parsedPackage['name'];
const moduleData = modulesManifest[packageName];

// We don't want to modify the "package.json" if we guessed the entry-point
// paths and there is a custom "package.json" for that package already. Module
// data will be only undefined if the package name comes from a non-generated
// "package.json". In that case we want to leave the file untouched as well.
if (!moduleData || moduleData.guessedPaths && !isGeneratedPackageJson) {
// If a package json file has been discovered that does not match any
// module in the manifest, we report a warning as most likely the target
// is configured incorrectly (e.g. missing `module_name` attribute).
if (!moduleData) {
// Ideally we should throw here, as we got an entry point that doesn't
// have flat module metadata / bundle index, so it may have been an
// ng_module that's missing a module_name attribute.
Expand All @@ -342,6 +348,19 @@ function main(args: string[]): number {
return JSON.stringify(parsedPackage, null, 2);
}

// If we guessed the index paths for a module, and it contains an explicit `package.json`
// file that already sets format properties, we skip automatic insertion of format
// properties but report a warning in case properties have been set by accident.
if (moduleData.guessedPaths && !isGeneratedPackageJson &&
hasExplicitFormatProperties(parsedPackage)) {
console.error('WARNING: `package.json` explicitly sets format properties (like `main`).');
console.error(
' Skipping automatic insertion of format properties as explicit ' +
'format properties are set.');
console.error(' Ignore this warning if explicit properties are set intentionally.');
return JSON.stringify(parsedPackage, null, 2);
}

// Derive the paths to the files from the hard-coded names we gave them.
// TODO(alexeagle): it would be better to transfer this information from the place
// where we created the filenames, via the modulesManifestArg
Expand Down Expand Up @@ -377,6 +396,12 @@ function main(args: string[]): number {
return [relativePath, dir, basename + '.js'].join('/');
}

/** Whether the package explicitly sets any of the format properties (like `main`). */
function hasExplicitFormatProperties(parsedPackage: {[key: string]: string}): boolean {
return Object.keys(parsedPackage)
.some(propertyName => knownFormatPackageJsonFields.includes(propertyName));
}

/** Creates metadata re-export file for a secondary entry-point. */
function createMetadataReexportFile(
entryPointName: string, metadataFile: string, packageName: string) {
Expand Down

0 comments on commit d5293d2

Please sign in to comment.