[@ngtools/webpack] Decoration of compiler.inputFileSystem breaks its API

[DorianGrey]

I've recently attempted to update one of my projects to webpack v4, in addition to a couple of other updates. One of them broke my build with a curious error:

TypeError: readFileFn is not a function

After debugging things a bit, I figured out that one of the plugins I'm using attempts to access compiler.inputFileSystem._readFile, which is present in the apply phase, but was no longer existing when it came to emit.
Playing a bit with the configuration, I was able to point out that this modification was made by the AngularCompilerPlugin, which is part of @ngtools/webpack. More precisely, it happens due to the decoration performed by it:

angular-cli/packages/@ngtools/webpack/src/angular_compiler_plugin.ts

Lines 549 to 553 in d1b22d0

	compiler.hooks.environment.tap('angular-compiler', () => {
	compiler.inputFileSystem = new VirtualFileSystemDecorator(
	compiler.inputFileSystem, this._compilerHost);
	compiler.watchFileSystem = new VirtualWatchFileSystemDecorator(compiler.inputFileSystem);
	});

After this, compiler.inputFileSystem._readFile is no longer available. Thus, the decoration breaks the API available on compiler.inputFIleSystem and makes plugins relying on it fail.

Versions

• No ng version, since it's a custom setup
• Node 8.10.0
• NPM 5.7.1
• Yarn 1.5.1
• @ngtools/webpack 1.10.2
• Ubuntu 16.04

Repro steps

• Clone repository: https://github.com/DorianGrey/ng-webpack-template/tree/webpack-4
• yarn
• yarn build

Observed behavior

The last command will fail with an error like this.

TypeError: readFileFn is not a function

(See the build output here: https://travis-ci.org/DorianGrey/ng-webpack-template/jobs/357274315)

Desired behavior

Decoration or encapsulation of any of these internal parts of webpack should not break their available API, regardless of potentially intended privacy (suppose the leading _ might tend in this direction).

Mention any other details that might be useful (optional)

I've also tried the latest beta version of the @ngtools/webpack plugin and made a cross-check against webpack v3, but neither made any difference.
Not sure if this is of any help or use, but the affected plugin in my case is the workbox-webpack-plugin, which changed it's implementation and behavior in v3.

https://github.com/GoogleChrome/workbox/blob/fa4d8b7dab2b0680c15d6057c12fd9ec9f0932c4/packages/workbox-webpack-plugin/src/inject-manifest.js#L150-L154

Suppose that's why this problem did not raise before.

[filipesilva]

I believe the _readFile method in the native Webpack inputFileSystem is a private-ish method. I say private-ish because JS doesn't really have private methods but it is common practice to prefix methods with _ to mark them as private.

With this in mind, there is no guarantee that it should exist in the inputFileSystem because it might have been decorated (as is the case here).

Decorating inputFileSystem is the recommended approach to intercept FS calls, according to the Webpack author (webpack/enhanced-resolve#98 (comment)). We've been through this in the past with ngtools/webpack because we weren't decorating it properly (#7169 and #7471).

I think workbox-webpack-plugin needs to also decorate the file system like we did to ensure compatibility between plugins.

[hansl]

Like @filipesilva said, _readFile is private. You should use readFile, which we do provide and is the proper public method to use.

[DorianGrey]

I'm not sure if using another decoration here makes sense or is useful - it would end up with a decoration of an already decorated inputFileSystem, given that compiler.inputFileSystem is simply replaced. Since the decoration does not have an enforced API, other instances attempting to decorate it might end up non-functional due to making assumptions about inputFileSystem that only hold true on the original version, not the decorated one - or vice versa.

However, I agree that relying on a private method isn't the best idea, and will open a corresponding issue on the workbox repository, and see if there is a better solution - esp. since the plugin only utilizes this method for reading a single file.

[clydin]

It's also important to note that the webpack filesystem abstractions are intended to be extendable and/or re-implementable. Webpack provides NodeJS versions which are used by default. These specifically have the following:

stat
statSync
readdir
readdirSync
readFile
readFileSync
readlink
readlinkSync

The plugin in question here appears to be assuming that webpack is using, and providing via inputFileSystem, the CachedInputFileSystem; which is not guaranteed.

[DorianGrey]

Suppose I can close this - the corresponding issue on the other repo has been closed due to a fix being merged recently.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}