Skip to content

feat(core): allow to throw on unknown elements in tests#45479

Closed
cexbrayat wants to merge 1 commit intoangular:mainfrom
cexbrayat:feat/throw-on-unknown-element
Closed

feat(core): allow to throw on unknown elements in tests#45479
cexbrayat wants to merge 1 commit intoangular:mainfrom
cexbrayat:feat/throw-on-unknown-element

Conversation

@cexbrayat
Copy link
Member

@cexbrayat cexbrayat commented Mar 30, 2022

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Feature

What is the current behavior?

In a test, when using an unknown element in a template, we just get a console error.

Issue Number: #36430

What is the new behavior?

Allows to provide a TestBed option to throw on unknown elements in templates:

getTestBed().initTestEnvironment(
  BrowserDynamicTestingModule,
  platformBrowserDynamicTesting(),
  { errorOnUnknownElements: true }
);

Does this PR introduce a breaking change?

The default value of throwOnUnknownElement is false, so this is not a breaking change.

  • Yes
  • No

@google-cla
Copy link

google-cla bot commented Mar 30, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

For more information, open the CLA check for this pull request.

@cexbrayat cexbrayat force-pushed the feat/throw-on-unknown-element branch 2 times, most recently from 68cd385 to 18af293 Compare March 30, 2022 17:39
@dylhunn dylhunn added the area: core Issues related to the framework runtime label Mar 30, 2022
@ngbot ngbot bot added this to the Backlog milestone Mar 30, 2022
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

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

@cexbrayat thanks for creating this PR! 👍
The changes look great, just left a few minor comments.

@cexbrayat cexbrayat force-pushed the feat/throw-on-unknown-element branch 7 times, most recently from 052f49f to 67ce006 Compare April 5, 2022 13:11
@cexbrayat cexbrayat force-pushed the feat/throw-on-unknown-element branch 3 times, most recently from 464a47e to ce828a7 Compare April 19, 2022 14:11
@cexbrayat cexbrayat marked this pull request as ready for review April 19, 2022 14:55
@cexbrayat cexbrayat force-pushed the feat/throw-on-unknown-element branch 2 times, most recently from 6a02152 to 6b556e3 Compare April 20, 2022 20:50
@AndrewKushnir AndrewKushnir added action: review The PR is still awaiting reviews from at least one requested reviewer target: major This PR is targeted for the next major release and removed state: WIP labels Apr 20, 2022
@AndrewKushnir AndrewKushnir modified the milestones: Backlog, v14-candidates Apr 20, 2022
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

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

@cexbrayat the changes look good, thanks Cedric!

We've discussed this change during the team sync and this looks like the approach we want to take. This approach allows us to enable this by default (in one of the next major versions) and use this mechanism for opt-out.

@pullapprove pullapprove bot requested review from AndrewKushnir and alxhub April 22, 2022 00:36
Copy link
Contributor

@jessicajaniuk jessicajaniuk left a comment

Choose a reason for hiding this comment

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

reviewed-for: public-api

Copy link
Contributor

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

Reviewed-for: public-api

@cexbrayat cexbrayat force-pushed the feat/throw-on-unknown-element branch 2 times, most recently from 4129e51 to 5e2a608 Compare April 29, 2022 18:08
@cexbrayat
Copy link
Member Author

We agreed with @AndrewKushnir that errorOnUnknownElements was probably a better name, as @atscott and @jelbourn suggested. I updated the PR to use errorOnUnknownElements instead of throwOnUnknownElements

Allows to provide a TestBed option to throw on unknown elements in templates:

```ts
getTestBed().initTestEnvironment(
  BrowserDynamicTestingModule,
  platformBrowserDynamicTesting(), {
    errorOnUnknownElements: true
  }
);
```

The default value of `errorOnUnknownElements` is `false`, so this is not a breaking change.
@cexbrayat cexbrayat force-pushed the feat/throw-on-unknown-element branch from 5e2a608 to 1d169e8 Compare April 29, 2022 18:23
@AndrewKushnir
Copy link
Contributor

AndrewKushnir commented Apr 29, 2022

Presubmit (note: this change would also require this update in g3) + TGP.

@AndrewKushnir AndrewKushnir added feature Label used to distinguish feature request from other issues action: presubmit The PR is in need of a google3 presubmit and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Apr 29, 2022
@AndrewKushnir AndrewKushnir added action: merge The PR is ready for merge by the caretaker and removed action: presubmit The PR is in need of a google3 presubmit labels Apr 30, 2022
@AndrewKushnir
Copy link
Contributor

FYI, the presubmit went well and I'm adding this PR to the merge queue.

@dylhunn
Copy link
Contributor

dylhunn commented May 2, 2022

This PR was merged into the repository by commit 6662a97.

@dylhunn dylhunn closed this in 6662a97 May 2, 2022
dylhunn added a commit to dylhunn/angular that referenced this pull request May 2, 2022
dylhunn added a commit that referenced this pull request May 2, 2022
@dylhunn dylhunn reopened this May 2, 2022
@dylhunn
Copy link
Contributor

dylhunn commented May 2, 2022

Rolled this back temporarily because I didn't see the g3 patch above. I will roll forward again and sync with the patch.

@dylhunn
Copy link
Contributor

dylhunn commented May 2, 2022

This PR was merged into the repository by commit e702caf.

@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 Jun 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime feature Label used to distinguish feature request from other issues target: major This PR is targeted for the next major release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants