Skip to content

feat(@schematics/schematics): add includeStyles option on component #18420

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
wants to merge 1 commit into from
Closed

feat(@schematics/schematics): add includeStyles option on component #18420

wants to merge 1 commit into from

Conversation

geromegrignon
Copy link
Contributor

There are several use cases where we create components knowing there will be no styles attached.
To keep them as simple as possible, there should be an option to create them without any styles/stylesUrl property into the @component metadata.

new Component schematic option

"includeStyles": {
      "description": "Specifies if the component should include styles",
      "type": "boolean",
      "default": true,
      "alias": "is"
    }

@alan-agius4
Copy link
Collaborator

@geromegrignon, can you please mention the use cases?

It would help us evaluate this request better. Thanks.

@fstodulski
Copy link

Using tailwind is a good example

@sjtrimble
Copy link
Contributor

The main use case for me was when you have a SCSS structure (like we have in AIO) you don't need individual stylesheets or inline styles at all on a component level basis.

I'm wondering if we can make this a global config setting instead?

@santoshyadavdev
Copy link
Contributor

The main use case for me was when you have a SCSS structure (like we have in AIO) you don't need individual stylesheets or inline styles at all on a component level basis.

I'm wondering if we can make this a global config setting instead?

Yes, we do have the exact use case, and we ended up deleting the scss files after generating the component.

@alan-agius4 alan-agius4 added the needs: discussion On the agenda for team meeting to determine next steps label Aug 1, 2020
@alan-agius4
Copy link
Collaborator

alan-agius4 commented Aug 1, 2020

I marked this for further discussion with the team to see if we’d want to add this option.

Side note: while I do understand that in some cases you might not want components styles, I think those should be minimal unless using a CSS framework where you don’t need write your own CSS. Than again probably you still might want some component specific styling.

From a performance perspective, my personal suggestion would to go for components styles instead of global styles (styles.css). Here’re some reasons:

  1. External stylesheets are in the critical path and render-blocking and hence should be kept as small as possible.
  2. Unlike global styles, components styles get lazy-loaded with the component itself.
  3. Better style encapsulation.
  4. In Issue Eliminate render-blocking resources #17966 we can see that even by having an empty style.css we start degrading the performance of our application.
  5. Lighthouse will also penalised use because large parts of the CSS file will be unused in a given page.

In addition to that I think it is also a better DX as it makes it easier to maintain component specific styles.

Do use the global stylesheet, but I see this to be used more for generic utility classes and browsers resets and normalisation styles. (Ie: truly global)

@geromegrignon
Copy link
Contributor Author

About use cases, i would add :

  • container components in a smart /dumb architecture (as their purpose is to provide data to a presentational component)
  • components whose purpose is to display components through ng-template or ng-content and delegating the styles to injected components

@alan-agius4 alan-agius4 self-assigned this Aug 6, 2020
@alan-agius4 alan-agius4 removed the needs: discussion On the agenda for team meeting to determine next steps label Aug 7, 2020
@alan-agius4
Copy link
Collaborator

Hi @geromegrignon, Thanks for your contribution.

We have discussed this during our team meeting and we decided that for the time being we'd like to keep this out for a number of reasons.

We'd like to re-evaluate all the options of our schematics. The component schematic currently has 21 options, 3 of which are related to styling --inline-styles, '--style', '--display-block'. Some of these options cannot be used together such as style and inline-styles.

Having so many options is also a maintain concern, additionally it makes each option less discoverable in the documentation and help page. Therefore, we'd like to evaluate the possibility of merging/removing some of these.

At the moment users can opt to not create external CSS files using --inline-styles option. Which should be a good enough alternative.

Thanks again!

@alan-agius4 alan-agius4 closed this Aug 7, 2020
@geromegrignon
Copy link
Contributor Author

Hi @alan-agius4, thanks for the response and the time invested in reviewing it.

@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, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants