Skip to content

Conversation

wKoza
Copy link
Contributor

@wKoza wKoza commented Feb 16, 2017

Since we use webpack, we don't have to use compileComponents(). Webpack transforms html and css file to inline template.

Copy link
Contributor

@deebloo deebloo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would also mean async() isn't necessary correct?

@wKoza
Copy link
Contributor Author

wKoza commented Feb 16, 2017

You are right, i think that it's not necessary.
@filipesilva , you are interesting with this PR ?

@hansl
Copy link
Contributor

hansl commented Feb 16, 2017

Hi @wKoza @deebloo ,

These should still be in there. You're right that when using the @ngtools/webpack plugin we don't need those (mostly because there's no fetch calls). However, there's no hard performance implication for having those and they ARE best practices for every test.

We aim at aligning very closely with the Angular official style guide which recommends calling those for every async test.

Please also note that even if we use webpack internally, we still try to hide webpack from the user. This uses an implementation detail that we could switch in the future (or the user decides to stop using the CLI) and all the tests would be rendered invalid.

TL;DR: this is not broken, this does not impact performance, it's best practice, so we don't need to change it.

@hansl hansl closed this Feb 16, 2017
@wKoza
Copy link
Contributor Author

wKoza commented Feb 17, 2017

Hi @hansl ,

We aim at aligning very closely with the Angular official style guide which recommends calling those for every async test.

Angular official says :

WebPack developers need not call compileComponents because it inlines templates and css as part of the automated build process that precedes running the test.

At which Angular official style guide you refer ?

@wKoza
Copy link
Contributor Author

wKoza commented Feb 17, 2017

@hansl

Furthermore, compileComponents () is asynchronous. So, we must use async () method like this:

  BeforeEach` (async (() => {
     TestBed.configureTestingModule ({
       Declarations: [BannerComponent], // declare the test component
     })
     .compileComponents (); // compile template and css
   }));

not

BeforeEach (() => {
     TestBed.configureTestingModule ({
       Declarations: [BannerComponent], // declare the test component
     })
    TestBed.compileComponents (); // compile template and css
   });

I will open a issue.

@jasedwards
Copy link

Hi, angular.io now specifically states that if you use angular-cli you don't need to use compileComponents. Can removing the usage of this in the component schematic be revisited?

https://angular.io/guide/testing#calling-compilecomponents

@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 13, 2019
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.

5 participants