Skip to content

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

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

Conversation

cexbrayat
Copy link
Member

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

…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 fix/remove-compile-components branch from edd7d22 to bb3cfe1 Compare July 21, 2020 11:49
@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 added the target: major This PR is targeted for the next major release label Aug 14, 2020
@alan-agius4 alan-agius4 requested a review from clydin August 27, 2020 07:47
@cexbrayat
Copy link
Member Author

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

@alan-agius4
Copy link
Collaborator

alan-agius4 commented Sep 29, 2020

Let me bring this up with the rest of the team in our next team meeting.

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

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.

@dgp1130 dgp1130 added needs: investigation Requires some digging to determine if action is needed and removed needs: discussion On the agenda for team meeting to determine next steps labels Oct 8, 2020
@dgp1130
Copy link
Collaborator

dgp1130 commented Jun 11, 2021

We've been looking through some older PRs and came across this one. Landing this largely depends on future of TestBed which we still don't have a great strategy for right now and likely won't for some time. It's unlikely this PR will be able to go anywhere without a better idea of how TestBed will evolve, and we don't have a timeline for when that will happen.

Whenever it does happen, we'll likely need to update schematics anyways and possibly change more than just compileComponents() so it makes to handle this as part of that change instead of removing compileComponents() preemptively or saving this PR until it can actually be merged. As a result, I'm closing this PR, I imagine we'll revist compileComponents() when the time is right and update the schematic accordingly.

@dgp1130 dgp1130 closed this Jun 11, 2021
@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 Jul 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs: investigation Requires some digging to determine if action is needed target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants