-
Notifications
You must be signed in to change notification settings - Fork 12k
feat: webpack plugin to support AoT natively #2333
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
Conversation
We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm. |
Both @robwormald and I are Google employees and therefore agree to the CLA. |
3550f17
to
20367f4
Compare
this.lazyRoutes = lazyModules.reduce((lazyRoutes: any, lazyModule: any) => { | ||
lazyRoutes[`${lazyModule}.ngfactory`] = path.join( | ||
path.resolve(process.cwd(), this.angularCompilerOptions.genDir), | ||
'app', lazyModule + '.ngfactory.ts' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i realized this is hardcoded and would fail for anything other than app
- should be I guess the relative path between the ngfactory directory and the app's root.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
2be135e
to
d6fbf39
Compare
42df2c8
to
6b0d77e
Compare
|
||
// Super simple TS transpiler loader for testing / isolated usage. does not type check! | ||
export function ngcLoader(source: string) { | ||
const plugin = this._compilation._ngToolsWebpackPluginInstance as NgcWebpackPlugin; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may be fine for now and once we drop webpack v2. I'm not sure when we are going to start giving deprecation warnings though for trying to access _compilation
.
cc @sokra when he gets back from vacation. Maybe we need to have a way to supress warnings of these deprecations of this._compilation
being unavailable from loaders on a per-plugin basis. That way this usage is acceptable for now, but in the future we can tail it off.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really want to make a point that we need access to compilation in many cases, inside the loaders.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or at least access to plugins, yes.
|
||
|
||
|
||
export class WebpackResourceLoader implements ResourceLoader { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since I haven't looked at ResourceLoader
I don't really know, but since this replaces angular2-template-loader and family, we probably are going to end up with some sad folks because they can't use sass/less/styl inside of components. But, at the cost of AoT, first pass with room to refactor, its justifiable. Just means there is a lot more responsibility on the ResourceLoader to handle all these things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If that is the case I would suggest making AOT a separate option for build because I do see that breaking a LOT of peoples builds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TheLarkInn I don't know what you're talking about, this was a requirement from the design phase. This resource loader compiles the resources asked for the component and as such will use the loaders.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup it was misunderstanding until I loaded it and tried myself. It makes perfect sense what you are doing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course the regex but you said you were going to get to that.
1ae9b65
to
4989a45
Compare
{ test: /\.scss$/, loaders: ['raw-loader', 'sass-loader'] }, | ||
{ test: /\.css$/, loader: 'raw-loader' }, | ||
{ test: /\.html$/, loader: 'raw-loader' }, | ||
{ test: /\.ts$/, loader: '@ngtools/webpack' } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We thinking about changing name of loader for the future? Not really worried but I'm sure theres some post merge 🚲 shedding to be done haha.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess since this is the test app, its really not a big deal at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not thinking of changing it, but we could. What were you thikning of?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure yet, this is fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You guys are killing it! Looking forward to this very much |
Also adding a new package, webpack, which is a plugin and loader for webpack that adds support for AoT. It is behind a `--aot` flag in the CLI that is supported by build and serve.
CLAs look good, thanks! |
Does this work with lazy loading? it is not generating chunk.js file either. Also I cannot find any class related to the Module in main.bundle.js |
How would I use this? There's no documentation in the commits as far as I can see? |
@rolandoldengarm use |
Also adding a new package, webpack, which is a plugin and loader for webpack that adds support for AoT. It is behind a `--aot` flag in the CLI that is supported by build and serve.
Also adding a new package, webpack, which is a plugin and loader for webpack that adds support for AoT. It is behind a `--aot` flag in the CLI that is supported by build and serve.
While looking at this push: d296778 Does it mean it will be offically supported in the next version of Angular-cli version 17? Unless somebody wants to pull master ? thanks, |
Is there any documentation available? I've been trying to get it to work for some time now. It doesn't build my lazily loaded routes and complains about the ngfactory files not found. Are there any changes for bootstrapping the app? |
@achimha all changes needed should be done in memory. We don't yet have documentation but basically you can use the |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
No description provided.