Skip to content

Add possibility to use webpack with an alternative file system #9089

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
nicojs opened this issue Jan 4, 2018 · 17 comments
Closed

Add possibility to use webpack with an alternative file system #9089

nicojs opened this issue Jan 4, 2018 · 17 comments
Labels
area: @ngtools/webpack feature Issue that requests a new feature needs: investigation Requires some digging to determine if action is needed
Milestone

Comments

@nicojs
Copy link
Contributor

nicojs commented Jan 4, 2018

Repro steps

Create a new project with ng new <project>. Cd in the directory and create a new file called "main.js" with the following content:

// main.js
const path = require('path');
const webpack = require('webpack');

const angularCliConfig = require('./.angular-cli');
const getAppFromConfig = require('@angular/cli/utilities/app-utils').getAppFromConfig;
const WebpackTestConfig = require('@angular/cli/models/webpack-test-config').WebpackTestConfig;

const appConfig = getAppFromConfig(undefined);

const testConfig = Object.assign({
  environment: 'dev',
  codeCoverage: false,
  sourcemaps: false,
  progress: true,
  preserveSymlinks: false,
});

const webpackConfig = new WebpackTestConfig(testConfig, appConfig).buildConfig();
delete webpackConfig.entry.styles;
webpackConfig.devtool = false;

const compiler = webpack(webpackConfig);
const fs = compiler.inputFileSystem;
const originalReadFile = fs.readFile;
fs.readFile = function (name, optionalArgs, callback) {
  if (name === path.resolve(__dirname, 'src', 'app', 'app.component.ts')) {
    if (!callback) {
      callback = optionalArgs;
    }
    callback(null, `alert('hello world!')`);
  } else {
    originalReadFile.apply(fs, [name, optionalArgs, callback]);
  }
}
compiler.run(() => {
  console.log('Done');
});

Run the "main.js" file.

Example project can be cloned here: https://github.com/nicojs/angular-cli-with-alternative-filesystem

Observed behavior

The original content from disk is bundled. The "main.bundle.js" file contains:

var AppComponent = (function () {
    function AppComponent() {
        this.title = 'app';
    }
   // ...
}());

Desired behavior

I would expect the 'main.bundle.js' to contain the alert('hello world') content as read from the input file system (or a compiler compiler error in this case...).

The use case here is that we want to use the angular webpack compiler with an in-memory filesystem. This is used by Stryker (https://stryker-mutator.github.io) to transpile input files in-memory.

Mention any other details that might be useful (optional)

We've looked into the problem and believe it lies within the 'WebpackCompilerHost.ts' file here:

this._delegate = ts.createCompilerHost(this._options, this._setParentNodes);

It creates a typescript compiler host which in turn uses the actual file system rather than passing through the webpack input file system.

Versions

ng version

    _                      _                 ____ _     ___
   / \   _ __   __ _ _   _| | __ _ _ __     / ___| |   |_ _|
  / △ \ | '_ \ / _` | | | | |/ _` | '__|   | |   | |    | |
 / ___ \| | | | (_| | |_| | | (_| | |      | |___| |___ | |
/_/   \_\_| |_|\__, |\__,_|_|\__,_|_|       \____|_____|___|
               |___/

Angular CLI: 1.6.3
Node: 8.2.1
OS: win32 x64
Angular: 5.1.2
... animations, common, compiler, compiler-cli, core, forms
... http, language-service, platform-browser
... platform-browser-dynamic, router

@angular/cli: 1.6.3
@angular-devkit/build-optimizer: 0.0.36
@angular-devkit/core: 0.0.22
@angular-devkit/schematics: 0.0.42
@ngtools/json-schema: 1.1.0
@ngtools/webpack: 1.9.3
@schematics/angular: 0.1.11
@schematics/schematics: 0.0.11
typescript: 2.4.2
webpack: 3.10.0
@nicojs nicojs changed the title Add possibility to use an alternative file system Add possibility to use webpack with an alternative file system Jan 4, 2018
@filipesilva
Copy link
Contributor

We do use a virtual file system ourselves, and I'm worried this might be a chicken-and-egg problem.

In here you can see how we decorate the webpack filesystem.

The VirtualFileSystemDecorator checks if our WebpackCompilerHost contains a file and serves it instead. It also contains VirtualWatchFileSystemDecorator to handle watch mode.

We need to do this because the Angular compiler generates files in memory too. I'm not sure how to get around this, the whole plugin is based on having the virtual compiler host.

But it seems to be you don't particularly care about having the virtual file system per se. I think you care about being able to modify files. With that in mind an alternative could be to expose an API to add TS transformers.

We use TS transformers (usage here, and all of them in here) to modify TS code.

Would such an API suffice? I'm not sure if this is something we can expose but before checking I want to know if it would work at all.

@nicojs
Copy link
Contributor Author

nicojs commented Jan 4, 2018

We do use a virtual file system ourselves, and I'm worried this might be a chicken-and-egg problem.

I see... It might be an idea to not override the inputFileSystem inside the webpack plugin. Instead just write the typescript compiler output to the inputFileSystem and make sure it is replaced by a file system implementation of your choosing when the webpack instance is instantiated in the karma plugin and when running ng serve. I think this is more the "webpack way of doing things". It would solve our problem as well, but it might be a pretty big redesign on your part.

I think you care about being able to modify files. With that in mind an alternative could be to expose an API to add TS transformers.

You are correct, we are only interested in changing the input files. Using a transformer actually works 👍 . Its quite a bit of code, you can see it here. If we would use this transformer api, i would like to see that it being hoisted to some kind of public api.

In the end I would still prefer to do things the webpack way (using inputFileSystem). That way we won't have to make an exception for the angular webpack plugin, we could just handle angular as we would any other webpack plugin.

@nicojs
Copy link
Contributor Author

nicojs commented Jan 4, 2018

I've been thinking. I'm not sure if the transform strategy will work in the long run because the angular webpack plugin will still not know that the file changed in-memory, as its watching the files on disk. It might not be fooled that easy and skip transpiling all together using the _emitSkipped boolean. Don't you think?

@filipesilva
Copy link
Contributor

I'd be interested in doing things 'the webpack way' and writing to the TS output to inputFileSystem. At the time when I looked into it, I did not find a straightforward way of doing it though. It would simplify things on our webpack plugin as well.

The inputFileSystem doesn't have write methods I think but they can be added to the decorator. I once spoke with @sokra about this and he recommended that I made a webpack plugin for a real virtual file system (based on webpack/webpack#5824 and https://github.com/filipesilva/webpack-virtual-module-rebuild/blob/master/virtual-files-plugin.js).

There's a couple of things that are not very straightforward, mostly regarding the purpose of a virtual file system decorator in webpack.

For Angular CLI, the purpose is to add/alter files before webpack actually runs. We need this because the kind of TS compilation we use in Angular requires requires type checking (thus is 'project' wide) and also creates files. So we need those files to all be there before webpack loads and bundles files. I'm under the impression that Stryker needs to do something similar.

So the virtual file system would need to:

  • decorate webpacks input file system
  • detect file changes
  • notify non-webpack consumers sequentially
    • each consumer can modify the existing files
  • notify webpack only at the end of everything that changed

This is somewhat subversive of Webpack to be honest. But I don't know of other ways to do it.

I should mention that the CLI team is looking at having a solid virtual FS as part of @angular-devkit: angular/devkit#328. This is not finalized yet, and unrelated to Webpack itself.

There's another problem with this approach though... our plugin actually needs an active webpack compilation (see here and here). We need it to have webpack process resources (html and css/sass/less/stylus files) and hand them over to the Angular Compiler.

This complicates things really. We want webpack to actually create a compilation but just wait there while we do stuff. And the way webpack creates a compilation is by getting notified by the file watcher that stuff changed. This is the real reason that has blocked me from looking more into the virtual file system.

Allowing for this sort of functionality changes the design of the virtual file system. It now needs to:

  • decorate webpacks input file system
  • detect file changes
  • notify webpack
  • allow consumers to intercept webpack file requests/lookups

Which at this point just sounds like duplicating Webpack's loader system. But creating a new file during loader operation would probably trigger a rebuild. Maybe there's a way to update the FS, not trigger the watcher, but still tell webpack that file has changed and needs to be read again (instead of using the cached version).

This is where I stopped thinking about the topic much more in the past. We're interested in this from the CLI point of view but it also isn't very high priority because things work right now.

@nicojs
Copy link
Contributor Author

nicojs commented Jan 8, 2018

Wow, the angular cli build is even more complicated than I thought. Is it fair to say you are using webpack in ways that it is not designed for?

Why not moving the angular compiler entirely out of the webpack bundeling logic? By that, I mean: using the angular compiler to transpile and gather all needed in-memory files (transpile typescript, gather .html and css files). Next, use that to supply files to webpack to bundle it together. You will still need to decorate the inputFileSystem and watchingFileSystem to also read and trigger when an in-memory file is changed, but I believe it would still be much simplified in the end. An added benefit is that you are more decoupled from webpack and can chose another bundler in the future if you so desire.

This is much like the way we designed transpilers in Stryker. We allow transpilers to be 'chained'. For example: first do typescript transpilation (in-memory), next bundle them together using webpack (in-memory). The output of the one transpiler will be the input of the next one. The output of the last configured transpiler is the input for the test runner.

For now, I think we've investigated this angle enough. I think we cannot use the AngularWebpackPlugin in Stryker. We'll fallback on a completely separate build process based on https://github.com/Archcry/stryker-angular-extra by @Archcry, unless you have some other ideas.

This is where I stopped thinking about the topic much more in the past. We're interested in this from the CLI point of view but it also isn't very high priority because things work right now.

I understand that. Shall I close this issue for now?

@filipesilva
Copy link
Contributor

I have considered how a setup that only uses webpack as a bundler proper would look. There would be some duplication or resource (html/css) and assets (other files and images) logic and caching at least, and probably a duplicated pipeline (e.g. component css vs global css) which would need to be maintained in parallel, something we've done in the past and it proved to be hard to keep parity.

So the pros and cons would need to be weighted.

What you describe as the Stryker transpiler design is almost a one-to-one match to loaders in webpack though. I suppose there's much more to it but on a high level it seems similar.

I think I'd stop short of saying Webpack wasn't designed for the use we're giving it. The APIs are there, but they were not easy to find or use at all. I would definitely say we use Webpack in ways that make it very uncomfortable though.

I'd like to keep the issue open if you don't mind. There's valuable information here. It's a uncommon usecase (two plugins trying to decorate the InputFileSystem) but I would like this to be addressed in the future.

I think a good virtual file system plugin for webpack would be a worthwhile endeavour that would benefit the community, especially for advanced usecases like the Angular Compiler and Stryker.

I should mention that I'm not against work on this being done, but rather just wanted to explain why we didn't work on it further.

I don't have other better ideas for the integration you're proposing, no. It seems like you have something put together that works, and to do the virtual file system overhaul we're discussing here would take a while.

@nicojs
Copy link
Contributor Author

nicojs commented Jan 8, 2018

There would be some duplication or resource (html/css) and assets (other files and images) logic and caching at least, and probably a duplicated pipeline (e.g. component css vs global css) which would need to be maintained in parallel, something we've done in the past and it proved to be hard to keep parity.

Why would there be duplication? The way I see it, the angular cli would watch all interested files in the /app folder. Any changes on disk trigger a new angular compiler build. The changed files would reflect in the virtual filesystem you provide to webpack.

Some examples of the watch functionality functionality:

  • Changing app/app.component.ts: angular cli compile -> app.component.js -> webpack -> main.bundle.js
  • Changing app/app.component.css: angular cli pass through -> app.component.css -> webpack -> main.bundle.js
  • Changing style.css: webpack -> inline.bundle.js (or something).

I think a good virtual file system plugin for webpack would be a worthwhile endeavour that would benefit the community, especially for advanced usecases like the Angular Compiler and Stryker.

We are now using a hybrid we've created ourselves based on memory-fs and the node file system. It works for use case. Out of curiosity, why don't you use that one?

@filipesilva thanks for all the help. I'm looking forward to any updates in the future. I will not close this issue for now.

@filipesilva
Copy link
Contributor

Well, the Angular Compiler proper cares about TS files and nothing else. It asks the file system, or a given resource loader, for html/css/sass/etc.

So imagine that I have ./src/assets/image.png. It is used both in a global stylesheet, unrelated to the Angular app, and also on a Angular component stylesheet. Both stylesheets are SASS.

h1 {
  background-image: url('./assets/image.png')
}

The Angular compiler will ask for the component stylesheet and something will need to process the sass file and decide what to do with the image. The generated component code will have the style inlined somewhere.

Let's say it will not be inlined. The path will need to be rewritten to whatever will actually be available at runtime. So instead of './assets/image.png' you would probably want a processed image, and a fingerprinted filename. Maybe './assets/image.123456.png'.

Meanwhile whatever processes the global stylesheet has to do the same. But that's separate from the Angular Compiler really.

So you need a separate SASS compiler that you can configure with that kind of thing. You'll need a hashing function, and some knowledge of how the resource paths need to be renamed. You'll also need caching so rebuilds aren't slow.

These are all things that webpack knows how to do very well and not doing them within webpack while still using webpack is somewhat wasteful.

Doing component css outside of webpack while doing global styles inside webpack needs a lot of option synchronization and communication to keep consistency. It would be hard to tell both sass processors what the hashes should be and that a certain file is an asset in both.

It gives a lot more control though. But I think it's still possible to do it within webpack and not have all the trouble of a separate pipeline.

I think we used our own in-memory file system because we created it from the TypeScript host down, and the author at the time (@hansl) was experienced in creating virtual TS hosts.

BTW I pinged @TheLarkInn about this thread. Maybe he knows of more projects that need to do things similar to what we need, and we could coordinate efforts for a more robust virtual file system plugin in Webpack.

@sanderkoenders
Copy link

sanderkoenders commented Jan 8, 2018

We currently use the approach described by Angular to transpile input files using Webpack in Stryker, the stryker-webpack-angular-preset is based on this (https://angular.io/guide/webpack). Can we assume this will always work for any project that is developed within the Angular eco system?

@filipesilva
Copy link
Contributor

@Archcry I'm sorry to be the bearer of bad news, but that guide is old and I think is deprecated. I doesn't appear on the sidebar anymore I think.

I know that a lot of projects use that approach though, especially for faster rebuilds at the expense of more involved typechecking and transformations.

The one we officially support and maintain is using @ngtools/webpack (https://github.com/angular/angular-cli/blob/master/packages/@ngtools/webpack/README.md).

But from this conversation with @nicojs I don't think integrating it as is with Stryker is an easy endeavour.

@clydin
Copy link
Member

clydin commented Jan 8, 2018

So the underlying issue here appears to be that the internal typescript compiler is performing direct filesystem access. This could be remedied by removing the use of the delegate default TS CompilerHost and implementing the filesystem functionality in the WebpackCompilerHost in terms of the input file system. This assumes that any custom input file system provides both sync and async calls.

@filipesilva
Copy link
Contributor

@clydin I don't think that would work because we already decorate the input file system to intercept calls to the compiler host (see #9089 (comment)).

Although... maybe that could work and had a way to say that only auto generated files should be intercepted by our decorator. That might work.

@nicojs
Copy link
Contributor Author

nicojs commented Jan 8, 2018

@clydin yes, I think so. So you should provide you're own file IO functions here:

this._delegate = ts.createCompilerHost(this._options, this._setParentNodes);

Just a heads up: we should be able to override the inputFileSystem before we call compiler.run(...) (or compiler.watch(...) on the webpack compiler.

@filipesilva filipesilva added area: @ngtools/webpack feature Issue that requests a new feature labels Oct 8, 2019
@ngbot ngbot bot added this to the Backlog milestone Oct 8, 2019
@kyliau kyliau added needs: discussion On the agenda for team meeting to determine next steps triage #1 labels May 26, 2020
@dgp1130 dgp1130 added needs: investigation Requires some digging to determine if action is needed and removed needs: discussion On the agenda for team meeting to determine next steps labels Jun 26, 2020
@benwinding
Copy link

But it seems to be you don't particularly care about having the virtual file system per se. I think you care about being able to modify files. With that in mind an alternative could be to expose an API to add TS transformers.

Is it still possible to transform files before the AngularCompilerPlugin touches them?

Just like this issue, we'd like to modify typescript files and Add @NgModules through a typescript transform BEFORE the AngularCompilerPlugin processes them. But I can't seem to work out a way to do that...

@benwinding
Copy link

Don't worry, I added a new issue #19328

@clydin clydin removed their assignment Nov 16, 2020
@alan-agius4
Copy link
Collaborator

Thanks for reporting this issue. This issue is now obsolete due to changes in the recent releases. Please update to the most recent Angular CLI version.

If the problem persists after upgrading, please open a new issue, provide a simple repository reproducing the problem, and describe the difference between the expected and current behavior.

@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jul 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: @ngtools/webpack feature Issue that requests a new feature needs: investigation Requires some digging to determine if action is needed
Projects
None yet
Development

No branches or pull requests

9 participants