Skip to content

@import statements are not moved to the top of the file anymore #9267

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
tommueller opened this issue Jan 18, 2018 · 11 comments
Closed

@import statements are not moved to the top of the file anymore #9267

tommueller opened this issue Jan 18, 2018 · 11 comments
Labels
P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent severity5: regression type: bug/fix

Comments

@tommueller
Copy link

tommueller commented Jan 18, 2018

Versions


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

Angular CLI: 1.6.4
Node: 8.9.4
OS: linux x64
Angular: 5.2.0
... animations, common, compiler, compiler-cli, core, forms
... http, platform-browser, platform-browser-dynamic
... platform-server, router

@angular/cdk: 5.0.4
@angular/cli: 1.6.4
@angular/material: 5.0.4
@angular-devkit/build-optimizer: 0.0.38
@angular-devkit/core: 0.0.25
@angular-devkit/schematics: 0.0.48
@ngtools/json-schema: 1.1.0
@ngtools/webpack: 1.9.4
@schematics/angular: 0.1.13
@schematics/schematics: 0.0.13
typescript: 2.4.2
webpack: 3.10.0

Repro steps

  • import a css file using the angular-cli.json styles field
  • build project using angular cli version 1.5.0
  • build project using angular cli version 1.6.4

Observed behavior

This is a regression bug. Building the project using cli version 1.5.0 pushes the @import ... statement of the imported css to the top of the generated *.bundle.css-file. Doing the same with cli version 1.6.4 does not push the import up and hence the browsers don't import the css file.

Desired behavior

Also version 1.6.4 should push the @import's to the top of the file, so that browsers import the dependencies.

@clydin
Copy link
Member

clydin commented Jan 19, 2018

Can you provide the content of the css file described in the above repro steps?

@tommueller
Copy link
Author

I cannot paste the whole file, as it is very large and not open source but I think the important point is that it starts like this:

@import url('https://fonts.googleapis.com/css?family=Material+Icons');
html {
   height: 100%
}
...

@hansl hansl added type: bug/fix P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent severity5: regression labels Jan 24, 2018
@tommueller
Copy link
Author

In order to get my CI back working, I ran the ng build command downgrading versions step by step to find a CLI version that I can use, here is the results:

  • from 1.6.2 to 1.6.3: comments in bundle.css are not removed anymore, but import statement is still on top of file
  • from 1.6.3 to 1.6.4: import statement is not pushed to top of file anymore

Concerning the css-bundling I did not find any changes in 1.6.5 and 1.6.6.

I will pin to 1.6.2 until this is resolved.

@bampakoa
Copy link
Contributor

bampakoa commented Jan 31, 2018

I can confirm that it also happens in 1.6.7

@clydin
Copy link
Member

clydin commented Feb 5, 2018

We'll definitely look into this. However, it's important to note that adding the fonts via absolute URL imports will have performance implications on the application. The browser will not be aware that it needs to fetch the assets until it parses the stylesheet. This can cause a delay of first render. Adding them to index.html allows the browser to become more quickly aware of the need to fetch and improve visual performance for the end user.

As an example for index.html:

<link href="https://fonts.googleapis.com/icon?family=Material+Icons" rel="stylesheet">

@bampakoa
Copy link
Contributor

bampakoa commented Feb 6, 2018

@clydin Thanks for the tip. @tommueller I noticed that the guide for Material fonts has changed from version 1.4.0 and onwards. Version 1.1.0 of the Angular Material wiki storie stated to add them in styles.css but in the Angular Material official website they now suggest to do it in index.html https://material.angular.io/guide/getting-started#step-6-optional-add-material-icons

@hansl hansl unassigned clydin Feb 6, 2018
@tommueller
Copy link
Author

@clydin Thanks for the reply. I am aware that it is better to not use css imports. For now we use @import just for the font-imports in our internal css-framework. I still hesitate about removing it because it is a nice feature of a framework, when it only needs to be added to the page and resolves it's dependencies automatically ( I think). I will reconsider this, though.
Regardless of this best practice discussion I think this bug should be fixed.

@PTC-JoshuaMatthews
Copy link

@clydin Any update on this? We use a bootstrap theme that has the font prebuilt into the css. Modifying the theme isn't really good practice. Would be nice if prod build could handle it even if there is performance hit.

@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.

@marcosavila
Copy link

marcosavila commented Oct 16, 2018

@alan-agius4 I'm having the same issue after update Angular CLI to 6.2.5

And seems that there are other guys that have the same issue. You can check it here: #10855

Have you in mind any kind of workaround for this?

@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 Sep 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent severity5: regression type: bug/fix
Projects
None yet
Development

No branches or pull requests

7 participants