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

CSS files in library modules are not autoprefixed #11480

Closed
kayahr opened this Issue Jul 6, 2018 · 13 comments

Comments

Projects
None yet
5 participants
@kayahr

kayahr commented Jul 6, 2018

Bug Report or Feature Request (mark with an x)

- [x] bug report -> please search issues before submitting
- [ ] feature request

Area

- [x] devkit
- [ ] schematics

Versions

$ node --version
v10.6.0

$ npm --version
6.1.0

$ lsb_release -a
Distributor ID:	Debian
Description:	Debian GNU/Linux 9.4 (stretch)
Release:	9.4
Codename:	stretch

$ ng -v
Angular CLI: 6.0.8
Node: 10.6.0
OS: linux x64
Angular: 6.0.7
... animations, common, compiler, compiler-cli, core, forms
... http, language-service, platform-browser
... platform-browser-dynamic, router

Package                            Version
------------------------------------------------------------
@angular-devkit/architect          0.6.8
@angular-devkit/build-angular      0.6.8
@angular-devkit/build-ng-packagr   0.6.8
@angular-devkit/build-optimizer    0.6.8
@angular-devkit/core               0.6.8
@angular-devkit/schematics         0.6.8
@angular/cli                       6.0.8
@ngtools/json-schema               1.1.0
@ngtools/webpack                   6.0.8
@schematics/angular                0.6.8
@schematics/update                 0.6.8
ng-packagr                         3.0.3
rxjs                               6.2.1
typescript                         2.7.2
webpack                            4.8.3

Repro steps

  1. Create a new demo app and a library project:

    $ ng new demo
    $ cd demo
    $ ng generate library demolib --prefix dl
    
  2. Add some old browsers to src/browserslist:

    IE 9-11
    safari 7
    
  3. Add the following CSS to src/app/app.component.css:

    .css-for-app-component {
      display: flex;
    }
    
  4. Change projects/demolib/src/lib/demolib.component.ts to use an external CSS file:

    ...
    @Component({
        ...
        styleUrls: ['./demolib.component.css']
    })
    ...
    
  5. Create projects/demolib/src/lib/demolib.component.css with the following content:

    .css-for-demolib-component {
      display: flex;
    }
    
  6. Include the demolib module in the application by changing src/app/app.module.ts:

    ...
    import { DemolibModule } from 'demolib';
    ...
    @NgModule({
        ...
        imports: [
            ...
            DemolibModule
        ],
        ...
    })
    ...
    
  7. Compile the library project and the app:

    $ ng build demolib
    $ ng build
    

The log given by the failure

Open the created dist/demo/main.js file and notice that the app CSS is prefixed but the library project CSS is not:

module.exports = ".css-for-app-component {\n  display: -webkit-flex;\n  display: -ms-flexbox;\n  display: flex;\n}\n"
DemolibComponent.decorators = [
        { type: _angular_core__WEBPACK_IMPORTED_MODULE_0__["Component"], args: [{
            selector: 'dl-demolib',
            template: "\n    <p>\n      demolib works!\n    </p>\n  ",
            styles: [".css-for-demolib-component{display:flex}"]
        },] },
];

Desired functionality

I would expect that all CSS in the project is autoprefixed according to the browserslist configuration.

Mention any other details that might be useful

Looks like the external CSS in the library project is embedded into the component by the compiler and it looks like the autoprefixer does only work with external CSS. So maybe this can be fixed by keeping the CSS external in the library and let the application build handle it? Or maybe the autoprefixer can be changed to also process embedded CSS? Then it would be possible to use embedded CSS in the app project, too, which is currently not autoprefixed.

However this issue is fixed it would be nice if it could be fixed in the application build (by autoprefixing the external CSS) and not in the library build (by running the autoprefixer there as well). Because in my opinion the application is responsible for deciding which browsers to support, not the library. The library can't know in which browsers it is used later. As long as the library is in the same project as the app this doesn't really matter but imagine you create an independent Angular component library, publish it on NPM and some app uses it as an external dependency. Now the App wants to support old IE and needs to prefix the CSS provided by the component library. This could work if the library project still provides the external CSS files which the app build can autoprefix before embedding it into the app bundle. Or the library project can embed the CSS into JavaScript and add some info to the generated metadata.json so the app build knows where the CSS can be found to pass it through the autoprefixer before writing it into the app bundle.

@kayahr kayahr changed the title from CSS in library modules are not autoprefixed to CSS files in library modules are not autoprefixed Jul 6, 2018

@alan-agius4

This comment has been minimized.

Collaborator

alan-agius4 commented Jul 6, 2018

The browserslist config needs to be in current or parent directories of the current process. Can you try to place it parallel to the root package.json?

@kayahr

This comment has been minimized.

kayahr commented Jul 6, 2018

Yes, when I copy the file to the root then the library project is also autoprefixed.

Well, I see how this will probably be the solution for this issue but unfortunately it is not really a good solution for me. As I tried to explain in the long text above in my opinion the auto-prefixing is the responsibility of the app build and should not be done in the library build. Because when I now compile the library with -ms- prefixes then an other application which also needs -webkit- prefixes can't use this library.

So it would be nicer if the libraries don't have to auto-prefix anything (as they usually also don't enforce the usage of any polyfills) and the app build (Which knows which browser must be supported and is also already responsible for the polyfill selection) instead also autoprefixes component styles from external libraries.

@alan-agius4

This comment has been minimized.

Collaborator

alan-agius4 commented Jul 6, 2018

So this is not bug.

Unfortunately, prefixing a component which is in a library from the application is not something that is possible at the moment and there are too many disadvantages of not shipping inlined css.

Some of these are;

  1. For consumers it will be harder to consume certain libraries, as they'd need to have a build pipeline for stylus, less, scss if this was used instead of plain css. (Unless the rendering of these is done at library level)
  2. You'd always need a bundler that can understand css so that it can be auto-prefixed. This will be break system-js users.
  3. You cannot simple use the UMD module and it works, such as on plunker.
  4. In the current Angular Package Format, there are 5 different module formats, es2015, es5, fesm5, fesm2015 and umd. Building with all these formats and re-write the templates will be rather complex.

I am not quite sure, that it is actually possible to inline css in a component after it was compiled at library level.

What I suggest is that as a library developer you should say that you will support a certain amount of browsers and you only prefix for those. This is even what big components libraries such as Angular Material & Ionic do.

@kayahr

This comment has been minimized.

kayahr commented Jul 6, 2018

Guess I have to live with that. So the only remaining minor problem is that ng new created the browserslist file in the src directory instead of the root directory? Maybe this task can be used to track this simple bug then.

@alan-agius4

This comment has been minimized.

Collaborator

alan-agius4 commented Jul 6, 2018

that browserslist is used for the application, the library build needs another browserslist configuration if you don't want to use the default.

@kayahr

This comment has been minimized.

kayahr commented Jul 6, 2018

So I have to place the application browserslist file in src and the library browserslist file in the root directory? This feels kinda wrong. I tried placing the file in projects, projects/demolib and projects/demolib/src, it doesn't work. It only works when the file is in the root directory.

So looks like currently it is only possible to define a default browserslist in the root and an application-specific browserslist in src. But it is not possible to define a library-specific browserslist file. The library always uses the default. So it is also not possible to define two libraries with different browserslist files.

I think it would be more intuitive if a library-specific file could be placed in projects/demolib or projects/demolib/src.

@alan-agius4

This comment has been minimized.

Collaborator

alan-agius4 commented Jul 6, 2018

alan-agius4 added a commit to alan-agius4/ng-packagr that referenced this issue Jul 6, 2018

alan-agius4 added a commit to alan-agius4/ng-packagr that referenced this issue Jul 6, 2018

alan-agius4 added a commit to alan-agius4/ng-packagr that referenced this issue Jul 6, 2018

dherges added a commit to ng-packagr/ng-packagr that referenced this issue Jul 17, 2018

dherges added a commit to ng-packagr/ng-packagr that referenced this issue Jul 17, 2018

@alan-agius4

This comment has been minimized.

Collaborator

alan-agius4 commented Jul 17, 2018

Fixed in ng-packagr v3.0.4 and v4.0.0-rc.4

@filipesilva

This comment has been minimized.

Member

filipesilva commented Aug 2, 2018

I can confirm that inline CSS (in apps or libraries) is never preprocessed by the application build system. Libraries, when built, always inline their CSS, so that ends up being unprocessable as well.

@peinearydevelopment

This comment has been minimized.

peinearydevelopment commented Aug 22, 2018

I'm still experiencing this issue. I placed the 'browserslist' file in parallel with the main 'package.json' file. There doesn't seem to be an ng-packagr version 3.0.4 or v4.0.0-rc.4. I tried to build the library with version 3.0.6 and 4.0.1 and none of the above combinations produced a compiled version of the css with autoprefixer having been applied.

What is the proper way to apply autoprefixer to a library build?

@alan-agius4

This comment has been minimized.

Collaborator

alan-agius4 commented Aug 22, 2018

The fix was applied in version 3.0.4 https://github.com/dherges/ng-packagr/blob/v3/CHANGELOG.md#304-2018-07-17

The browserlist should be located parallel to the package.json of the library.

Unfortunately, if there is bug it is not something that we at Angular-CLI can fix, and thus it is suggested to open an issue with ng-package, if the problem persists.

Thanks.

@peinearydevelopment

This comment has been minimized.

peinearydevelopment commented Aug 22, 2018

@alan-agius4 Thanks for the response! I'm happy to open an issue there, but I don't understand your comment. The issue you have linked doesn't seem to be relevant and looking at npm there doesn't seem to be any 3.0.4 version.

@alan-agius4

This comment has been minimized.

Collaborator

alan-agius4 commented Aug 22, 2018

Apologies I updated the link. That said you are right, it looks like for some reasons the 3.0.4 is only mentioned in the changelog and was not published to the npm registry.

But there should be the fix for that issue, so maybe you can also see how it’s being implemented from a consumer level

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment