fix(ngcc): ignore format properties that exist but are undefined (#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 #32052.
Fixes #32188
PR Close #32205
			
			
This commit is contained in:
		
							parent
							
								
									639b732024
								
							
						
					
					
						commit
						f8b995dbf9
					
				| @ -148,8 +148,8 @@ export function mainNgcc( | |||||||
|       const format = getEntryPointFormat(fileSystem, entryPoint, formatProperty); |       const format = getEntryPointFormat(fileSystem, entryPoint, formatProperty); | ||||||
| 
 | 
 | ||||||
|       // All properties listed in `propertiesToProcess` are guaranteed to point to a format-path
 |       // 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
 |       // (i.e. they are defined in `entryPoint.packageJson`). Furthermore, they are also guaranteed
 | ||||||
|       // among `SUPPORTED_FORMAT_PROPERTIES`.
 |       // to be among `SUPPORTED_FORMAT_PROPERTIES`.
 | ||||||
|       // Based on the above, `formatPath` should always be defined and `getEntryPointFormat()`
 |       // Based on the above, `formatPath` should always be defined and `getEntryPointFormat()`
 | ||||||
|       // should always return a format here (and not `undefined`).
 |       // should always return a format here (and not `undefined`).
 | ||||||
|       if (!formatPath || !format) { |       if (!formatPath || !format) { | ||||||
| @ -375,10 +375,10 @@ function getPropertiesToProcessAndMarkAsProcessed( | |||||||
| 
 | 
 | ||||||
|   const propertiesToProcess: EntryPointJsonProperty[] = []; |   const propertiesToProcess: EntryPointJsonProperty[] = []; | ||||||
|   for (const prop of propertiesToConsider) { |   for (const prop of propertiesToConsider) { | ||||||
|     // Ignore properties that are not in `package.json`.
 |     const formatPath = packageJson[prop]; | ||||||
|     if (!packageJson.hasOwnProperty(prop)) continue; |  | ||||||
| 
 | 
 | ||||||
|     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.
 |     // Ignore properties that map to the same format-path as a preceding property.
 | ||||||
|     if (formatPathsToConsider.has(formatPath)) continue; |     if (formatPathsToConsider.has(formatPath)) continue; | ||||||
| @ -390,10 +390,10 @@ function getPropertiesToProcessAndMarkAsProcessed( | |||||||
| 
 | 
 | ||||||
|   const formatPathToProperties: {[formatPath: string]: EntryPointJsonProperty[]} = {}; |   const formatPathToProperties: {[formatPath: string]: EntryPointJsonProperty[]} = {}; | ||||||
|   for (const prop of SUPPORTED_FORMAT_PROPERTIES) { |   for (const prop of SUPPORTED_FORMAT_PROPERTIES) { | ||||||
|     // Ignore properties that are not in `package.json`.
 |     const formatPath = packageJson[prop]; | ||||||
|     if (!packageJson.hasOwnProperty(prop)) continue; |  | ||||||
| 
 | 
 | ||||||
|     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.
 |     // Ignore properties that do not map to a format-path that will be considered.
 | ||||||
|     if (!formatPathsToConsider.has(formatPath)) continue; |     if (!formatPathsToConsider.has(formatPath)) continue; | ||||||
|  | |||||||
| @ -530,6 +530,67 @@ runInEachFileSystem(() => { | |||||||
|           typings: '0.0.0-PLACEHOLDER', |           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( |     function loadPackage( | ||||||
|  | |||||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user