angular / closure-demo

MIT License
115 stars 27 forks source link

use sass with Angular/Closure #11

Closed ianchi closed 6 years ago

ianchi commented 7 years ago

@alexeagle thank for your great work explaining how to try Closure Compiler with Angular. Could you hint the best way to integrate sass compiler into this building system not using Webpack. Could I preprocess everything with Rollup and then Closure or start using Gulp with some task to replace references of .scss files in components' styleUrls with compiled .css ones before calling ngc? Any other simpler way?

alexeagle commented 7 years ago

There's no build system here yet, so the obvious first step is just to invoke the sass compiler directly, and update the Angular components to reference the output .css files. (Angular CLI does some magic to update those references, which is hard to reproduce).

I'm working on open-sourcing the rules we use with http://bazel.build which includes SASS and that will be a better story.

I don't know about stitching together some other build tooling, but unless you're an enthusiast I don't recommend trailblazing some new combination of tools.

ianchi commented 7 years ago

OK, I'll try it manually first. Having Bazel rules would be great.

Tnks

ianchi commented 6 years ago

I'm having trouble making it work, I'm getting these messegas for all external imports:

build/app/app.component.js:5: ERROR - required "module$$angular$core" namespace never provided
import { Component, ViewEncapsulation } from '@angular/core';
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I'm using latest version of Google Closure Compiler and I do have them listed in the config, just as this repo:

--js node_modules/@angular/core/@angular/core.js
--js_module_root=node_modules/@angular/core
node_modules/@angular/core/src/testability/testability.externs.js

Any ideas?

alexeagle commented 6 years ago

@ianchi that doesn't seem related to using Sass. Can you narrow the problem down by making your setup more similar to the one in this repo until you see what's different? Or, post a repro that I can try.

ianchi commented 6 years ago

certainly not Sass related. I was trying to replicate the logic from this repo to my own project. Will double check what I missed.

ianchi commented 6 years ago

@alexeagle the problem is the new 20170626 version of Closure Compiler. This repo also breaks with the same error if you update to that version.

> closure-compiler-angular-bundling@1.0.0 closure /home/ianchi/dev/ianchi_wrt/closure-demo
> java -jar node_modules/google-closure-compiler/compiler.jar --flagfile closure.conf

built/app.module.js:1: ERROR - required "module$$angular$core" namespace never provided
import { NgModule } from '@angular/core';
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
alexeagle commented 6 years ago

Thanks for tracking it down. There was already a different breakage with an earlier version, https://github.com/google/closure-compiler/issues/2487 We have some work to do to fix closure compiler and add missing CI so it doesn't get broken again.

On Tue, Jul 18, 2017 at 8:09 AM Adrián Panella notifications@github.com wrote:

@alexeagle https://github.com/alexeagle the problem is the new 20170626 version of Closure Compiler. This repo also breaks with the same error if you update to that version.

closure-compiler-angular-bundling@1.0.0 closure /home/ianchi/dev/ianchi_wrt/closure-demo java -jar node_modules/google-closure-compiler/compiler.jar --flagfile closure.conf

built/app.module.js:1: ERROR - required "module$$angular$core" namespace never provided import { NgModule } from '@angular/core'; ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/angular/closure-demo/issues/11#issuecomment-316094788, or mute the thread https://github.com/notifications/unsubscribe-auth/AAC5I-zDSEEFrKhTKtBBvI7HSrUQDrHfks5sPMqQgaJpZM4OaiwI .

gregmagolan commented 6 years ago

@alexeagle I'm setting up a new Angular project and I'd like to use Angular with Bazel and Closure.

I can confirm that the angular/closure-demo build fails with the latest google-closure-compiler: 20170806.0.0

Originally at:
src/aot/app.module.ngfactory.ts:10: ERROR - required "module$$angular$core" namespace never provided
import * as i1 from '../app.module';
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

build/aot/app.module.ngfactory.js:11: 
Originally at:
src/aot/app.module.ngfactory.ts:14: ERROR - required "module$$angular$common" namespace never provided
import * as i5 from '@angular/platform-browser';
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

build/aot/app.module.ngfactory.js:12: 
Originally at:
src/aot/app.module.ngfactory.ts:15: ERROR - required "module$$angular$platform_browser" namespace never provided
export const AppModuleNgFactory:i0.NgModuleFactory<i1.AppModule> = i0.ɵcmf(i1.AppModule,
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

build/aot/basic.ngfactory.js:7: 
Originally at:
src/aot/basic.ngfactory.ts:10: ERROR - required "module$$angular$core" namespace never provided
import * as i1 from '../basic';
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

build/app.module.js:1: 
Originally at:
src/app.module.ts:1: ERROR - required "module$$angular$core" namespace never provided
import {Injectable, NgModule} from '@angular/core';
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

build/app.module.js:2: 
Originally at:
src/app.module.ts:2: ERROR - required "module$$angular$platform_browser" namespace never provided
import {BrowserModule} from '@angular/platform-browser';
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

build/basic.js:1: 
Originally at:
src/basic.ts:1: ERROR - required "module$$angular$core" namespace never provided
import {Component, Injectable} from '@angular/core';
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

build/main.js:1: 
Originally at:
src/main.ts:1: ERROR - required "module$$angular$platform_browser" namespace never provided
import {platformBrowser} from '@angular/platform-browser';
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The last working version is 20170409.0.0.

Any updates up this? Is there anything I can do to help out?

I have a repo up at https://github.com/gregmagolan/angular-clusure-build that I'm working from.

I've also checked out your Angular Bazel build example (https://github.com/alexeagle/angular-bazel-example) using the new bazelbuild/rules_typescript. What would be required to get that working with closure? Presumably, https://github.com/bazelbuild/rules_closure, would not work with Angular and TypeScript yet.

On a side note, I really like your clang-format for TypeScript suggestion. First time I've used it and its great!

alexeagle commented 6 years ago

@gregmagolan we raised the priority of this last week because Angular 5 will need some commits from closure-compiler so we have to get on the latest release. @thelgevold looked into it some. I think someone needs to run the Java debugger against the compiler to figure it out. Do you have some time to try?

For your other question, see https://github.com/bazelbuild/rules_typescript/issues/6

steveblue commented 6 years ago

Any progress on the ClosureCompiler side of things @alexeagle @thelgevold for supporting 5.0.0?

alexeagle commented 6 years ago

Angular 5 still relies on my fork of closure compiler, we are hoping to get a closure compiler release with everything working in time for Angular 5 RC.

steveblue commented 6 years ago

Will you please update the conf on your demo repo @alexeagle when the time comes so ClosureCompiler noobs like me can figure out at a glance how to update? I took your advice from ng-conf and looked into what @mlaval did, worked in similar logic to this build and it would be nice to have an example for RC to look at so I can quickly update and test the changes.

steveblue commented 6 years ago

@ianchi you can see an example of triggering sass compilation, then ngc, then closure compiler in this repo: https://github.com/steveblue/angular2-rollup

ianchi commented 6 years ago

Thanks @steveblue I did manage to make a manual build, but didn't find that much of size improvements. Since then, the "build optimizer" was added to angular-cli, and I ended with better results using Webpack, so I switched back to that.

alexeagle commented 6 years ago

re. not much of a size improvement - can you share any details? Was that with ADVANCED_OPTIMIZATIONS?

On Mon, Sep 18, 2017 at 2:55 PM Adrián Panella notifications@github.com wrote:

Thanks @steveblue https://github.com/steveblue I did manage to make a manual build, but didn't find that much of size improvements. Since then, the "build optimizer" was added to angular-cli, and I ended with better results using Webpack, so I switched back to that.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/angular/closure-demo/issues/11#issuecomment-330367682, or mute the thread https://github.com/notifications/unsubscribe-auth/AAC5I34e4GlRsRPsJY-JaUapH_jvVh4jks5sjubGgaJpZM4OaiwI .

ianchi commented 6 years ago

I mean comparing with the "ng build optimizer" builds. (which is a much simpler workflow). I don't have the numbers now, as I did the test about a month ago, and then abandoned Closure.

I dind't tried ADVANCED_OPTIMIZATIONS because I need all the API to be available for additional "plugins" that are compiled outside the main module, and lazy loaded on demand.

alexeagle commented 6 years ago

Ah, it's expected that anything less than ADVANCED_OPTIMIZATIONS is not really worth doing, it just does dead code removal which other tools also know how to do. If you had asked, we could have shown you how to make your plugins available outside the bundle by preventing the renaming of their properties, but I understand if you've moved on now.

On Mon, Sep 18, 2017 at 3:20 PM Adrián Panella notifications@github.com wrote:

I mean comparing with the "ng build optimizer" builds. (which is a much simpler workflow). I don't have the numbers now, as I did the test about a month ago, and then abandoned Closure.

I dind't tried ADVANCED_OPTIMIZATIONS because I need all the API to be available for additional "plugins" that are compiled outside the main module, and lazy loaded on demand.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/angular/closure-demo/issues/11#issuecomment-330372702, or mute the thread https://github.com/notifications/unsubscribe-auth/AAC5I-fzazSTRu3uYvQy2E_EPyRTjI0fks5sjuzEgaJpZM4OaiwI .

steveblue commented 6 years ago

Please show us how anyways @alexeagle. There are others who are using ClosureCompiler

alexeagle commented 6 years ago

It's a long topic...

The manual way is to create an externs file https://github.com/google/closure-compiler/wiki/Annotating-JavaScript-for-the-Closure-Compiler#externs that indicates which APIs should not be renamed.

Angular itself ships with these - for example an Angular app minified by closure must not rename the testability API, or else protractor tests wouldn't be able to use these hooks at runtime, so we ship https://github.com/angular/angular/blob/master/packages/core/src/testability/testability.externs.js Similar for zone.js, which includes externs since it's expected you'll load it as a separate script tag: https://github.com/angular/zone.js/blob/master/lib/closure/zone_externs.js

The better way is to generate externs with tsickle. We do this at google by marking external interfaces as declare interface I {} https://github.com/angular/tsickle#declare but this isn't hooked up with the ngc compiler right now. This feature will be part of ABC, so if you use Bazel to build your angular app or library, we will generate the externs.

ianchi commented 6 years ago

I was doing proof of concept of the size the production bundle would have. Right now I moved because the focus of the my Project is more in developing the core, but eventually I'll come back to optimize the bundle. I hope that by then the workflow is more streamlined, perhaps with Bazel.

steveblue commented 6 years ago

OK @alxhub I am indeed invoking the manual method for now with a file where externs are declared, nice to know we are on track with tsickle to provide the externs.

steveblue commented 6 years ago

I think this issue should be closed, the topic is not based on the original issue and besides, you can compile SCSS just fine down to CSS prior to ngc running.

alexeagle commented 6 years ago

thanks @steveblue