Skip to content

Commit

Permalink
fix(ngcc): ignore format properties that exist but are undefined (ang…
Browse files Browse the repository at this point in the history
…ular#32205)

Previously, `ngcc` assumed that if a format property was defined in
`package.json` it would point to a valid format-path (i.e. a file that
is an entry-point for a specific format). This is generally the case,
except if a format property is set to a non-string value (such as
`package.json`) - either directly in the `package.json` (which is unusual)
or in ngcc.config.js (which is a valid usecase, when one wants a
format property to be ignored by `ngcc`).

For example, the following config file would cause `ngcc` to throw:

```
module.exports = {
  packages: {
    'test-package': {
      entryPoints: {
        '.': {
          override: {
            fesm2015: undefined,
          },
        },
      },
    },
  },
};
```

This commit fixes it by ensuring that only format properties whose value
is a string are considered by `ngcc`.

For reference, this regression was introduced in angular#32052.

Fixes angular#32188

PR Close angular#32205
  • Loading branch information
elvisbegovic authored and ngdevelop-tech committed Aug 27, 2019
1 parent 49aa81c commit 727d995
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 8 deletions.
16 changes: 8 additions & 8 deletions packages/compiler-cli/ngcc/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,8 @@ export function mainNgcc(
const format = getEntryPointFormat(fileSystem, entryPoint, formatProperty);

// All properties listed in `propertiesToProcess` are guaranteed to point to a format-path
// (i.e. they exist in `entryPointPackageJson`). Furthermore, they are also guaranteed to be
// among `SUPPORTED_FORMAT_PROPERTIES`.
// (i.e. they are defined in `entryPoint.packageJson`). Furthermore, they are also guaranteed
// to be among `SUPPORTED_FORMAT_PROPERTIES`.
// Based on the above, `formatPath` should always be defined and `getEntryPointFormat()`
// should always return a format here (and not `undefined`).
if (!formatPath || !format) {
Expand Down Expand Up @@ -375,10 +375,10 @@ function getPropertiesToProcessAndMarkAsProcessed(

const propertiesToProcess: EntryPointJsonProperty[] = [];
for (const prop of propertiesToConsider) {
// Ignore properties that are not in `package.json`.
if (!packageJson.hasOwnProperty(prop)) continue;
const formatPath = packageJson[prop];

const formatPath = packageJson[prop] !;
// Ignore properties that are not defined in `package.json`.
if (typeof formatPath !== 'string') continue;

// Ignore properties that map to the same format-path as a preceding property.
if (formatPathsToConsider.has(formatPath)) continue;
Expand All @@ -390,10 +390,10 @@ function getPropertiesToProcessAndMarkAsProcessed(

const formatPathToProperties: {[formatPath: string]: EntryPointJsonProperty[]} = {};
for (const prop of SUPPORTED_FORMAT_PROPERTIES) {
// Ignore properties that are not in `package.json`.
if (!packageJson.hasOwnProperty(prop)) continue;
const formatPath = packageJson[prop];

const formatPath = packageJson[prop] !;
// Ignore properties that are not defined in `package.json`.
if (typeof formatPath !== 'string') continue;

// Ignore properties that do not map to a format-path that will be considered.
if (!formatPathsToConsider.has(formatPath)) continue;
Expand Down
61 changes: 61 additions & 0 deletions packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,67 @@ runInEachFileSystem(() => {
typings: '0.0.0-PLACEHOLDER',
});
});

it('should support removing a format property by setting it to `undefined`', () => {
loadTestFiles([
{
name: _('/ngcc.config.js'),
contents: `
module.exports = {
packages: {
'test-package': {
entryPoints: {
'.': {
override: {
fesm2015: undefined,
},
},
},
},
},
};
`,
},
{
name: _('/node_modules/test-package/package.json'),
contents: `
{
"name": "test-package",
"fesm2015": "./index.es2015.js",
"fesm5": "./index.es5.js",
"typings": "./index.d.ts"
}
`,
},
{
name: _('/node_modules/test-package/index.es5.js'),
contents: `
var TestService = (function () {
function TestService() {
}
return TestService;
}());
`,
},
{
name: _('/node_modules/test-package/index.d.js'),
contents: `
export declare class TestService {}
`,
},
]);

mainNgcc({
basePath: '/node_modules',
targetEntryPointPath: 'test-package',
propertiesToConsider: ['fesm2015', 'fesm5'],
});

expect(loadPackage('test-package').__processed_by_ivy_ngcc__).toEqual({
fesm5: '0.0.0-PLACEHOLDER',
typings: '0.0.0-PLACEHOLDER',
});
});
});

function loadPackage(
Expand Down

0 comments on commit 727d995

Please sign in to comment.