Skip to content

/ angular-cli

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

Jump to bottom

fix(@schematics/angular): remove compileComponents from component test schematic #18316

Open
wants to merge 1 commit into
base: master
from
Open

fix(@schematics/angular): remove compileComponents from component test schematic #18316

wants to merge 1 commit into from
+43 −95

Conversation

@cexbrayat
Copy link
Contributor

@cexbrayat cexbrayat commented Jul 21, 2020

compileComponents is not necessary when using the CLI and just adds boilerplate code, so we can remove it from the test schematic and make it independent from async/await (only place we would have it in the CLI generated code, and in most Angular apps).
@googlebot googlebot added the cla: yes label Jul 21, 2020
@cexbrayat
fix(@schematics/angular): remove compileComponents from component tes…
Loading status checks…
bb3cfe1 
…t schematic

`compileComponents` is not necessary when using the CLI (as the templates are inlined) and just adds boilerplate code. So we can remove it from the test schematic and make it independent from `async/await` (only place we would have it in the CLI generated code, and in most Angular apps).
@cexbrayat cexbrayat force-pushed the cexbrayat:fix/remove-compile-components branch from edd7d22 to bb3cfe1 Jul 21, 2020
@cexbrayat cexbrayat changed the title fix(@schematics/angular): remove async from component test schematic fix(@schematics/angular): remove compileComponents from component test schematic Jul 21, 2020
@clydin clydin added this to the V11-candidates milestone Aug 3, 2020
@alan-agius4 alan-agius4 requested a review from clydin Aug 27, 2020
@cexbrayat
Copy link
Contributor Author

@cexbrayat cexbrayat commented Sep 29, 2020

@alan-agius4 Now that we are in v11, do you think we can land this PR?
@alan-agius4
Copy link
Collaborator

@alan-agius4 alan-agius4 commented Sep 29, 2020 •
edited

Let me bring this up with the rest of the team in our next team meeting.
@dgp1130
Copy link
Collaborator

@dgp1130 dgp1130 commented Oct 1, 2020

We're planning some further investigation of the TestBed API and ways that we might be able to simplify it, particularly follow View Engine removal. Some better understanding there might help us remove the underlying need for compileComponents().

In our discussion today, the immediate concern is around portability. If a developer runs tests via direct Karma or Jest integration, they must use compileComponents() in order to handle the templateUrl parameter. If we remove this from the generated components, it could cause confusing errors which can only be fixed by special build plugins. Jest's Angular preset already seems to have one, though this is extra configuration that would be required to work with any external build tooling. It's more portable and far simpler from a tooling perspective to include compileComponents(), even if it may not be necessary for ng test use cases.

All that said, we'll need to revisit this once we have a better understanding of the future of TestBed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clydin clydin

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
PR target: master-only cla: yes need: investigation
Projects
None yet
Milestone
V11-candidates
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
@cexbrayat @alan-agius4 @dgp1130 @googlebot @clydin
You can’t perform that action at this time.