Skip to content
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

Invalid return type of "DebugElement.query" #22449

Open
stefanchrobot opened this issue Feb 26, 2018 · 6 comments · May be fixed by #48906
Open

Invalid return type of "DebugElement.query" #22449

stefanchrobot opened this issue Feb 26, 2018 · 6 comments · May be fixed by #48906
Labels
area: testing Issues related to Angular testing features, such as TestBed cross-cutting: types freq2: medium P4 A relatively minor issue that is not relevant to core functions state: confirmed type: bug/fix
Milestone

Comments

@stefanchrobot
Copy link

I'm submitting a...

[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request

Current behavior

DebugElement.query declares DebugElement as the return type.

Expected behavior

DebugElement.query declares DebugElement | null as the return type.

Minimal reproduction of the problem with instructions

let debugElement: DebugElement = ...;
let el = debugElement.query(By.CSS('not existent')).nativeElement;  // explodes

What is the motivation / use case for changing the behavior?

The result of DebugElement.query can actually be null.

The error should be caught by the compiler since the programmer should assert that the element was found. This is especially true for By.CSS predicate as it seems that the likelihood of typos and mistakes is pretty high.

The reason why this compiles without errors is that the return type of array indexing is T, not T | undefined. This seems to be the intended behaviour of the TypeScript compiler.

Environment

Angular version: latest
Browser: all
@ngbot ngbot bot added this to the needsTriage milestone Feb 26, 2018
@alexeagle alexeagle added the area: core Issues related to the framework runtime label Feb 26, 2018
@mhevery
Copy link
Contributor

mhevery commented Mar 29, 2018

Would you be awesome and create a PR for us?

@lacolaco
Copy link
Contributor

I can follow this up.

@pkozlowski-opensource pkozlowski-opensource removed the area: core Issues related to the framework runtime label Feb 28, 2020
@atscott atscott linked a pull request Aug 13, 2020 that will close this issue
14 tasks
@ngbot ngbot bot modified the milestones: Backlog, needsTriage Nov 10, 2020
@petebacondarwin petebacondarwin added the P4 A relatively minor issue that is not relevant to core functions label Nov 10, 2020
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Nov 10, 2020
@adam-marshall
Copy link

what is the status of this issue? noticed the PR was cancelled and we are now over 4 years on.

it causes an issue when trying to use the lint rule 'no unnecessary condition' - i.e. the user may know that query() may return null, but if they try and use optional chaining this contravenes the lint rule.

really the recommended advice should not be to deliberately throw, exceptions should be exceptional surely?

@rbirkgit
Copy link

We encountered issues with this in our unit tests. strict checking complains. Is this hard to fix? It feels like we just need to add | null as return type?

@JeanMeche
Copy link
Member

JeanMeche commented Feb 1, 2023

I just checked, debugElement.query is called 664 times alone in the angular repo.

A fixing of the type would require a migration schematics at least.

I might give it a try since have I done a similar one for #48039.

JeanMeche added a commit to JeanMeche/angular that referenced this issue Feb 1, 2023
`DebugElement.query()` can return null, let's reflect that on the return type.
This commit also provides a migration to insert non-null assertions after `DebugElement.query()` on existing code bases.

Fixes angular#22449
@JeanMeche JeanMeche linked a pull request Feb 1, 2023 that will close this issue
2 tasks
JeanMeche added a commit to JeanMeche/angular that referenced this issue Feb 2, 2023
`DebugElement.query()` can return null, let's reflect that on the return type.
This commit also provides a migration to insert non-null assertions after `DebugElement.query()` on existing code bases.

Fixes angular#22449
JeanMeche added a commit to JeanMeche/angular that referenced this issue Feb 2, 2023
`DebugElement.query()` can return null, let's reflect that on the return type.
This commit also provides a migration to insert non-null assertions after `DebugElement.query()` on existing code bases.

Fixes angular#22449
JeanMeche added a commit to JeanMeche/angular that referenced this issue Feb 2, 2023
`DebugElement.query()` can return null, let's reflect that on the return type.
This commit also provides a migration to insert non-null assertions after `DebugElement.query()` on existing code bases.

Fixes angular#22449
JeanMeche added a commit to JeanMeche/angular that referenced this issue Feb 3, 2023
`DebugElement.query()` can return null, let's reflect that on the return type.
This commit also provides a migration to insert non-null assertions after `DebugElement.query()` on existing code bases.

Fixes angular#22449
JeanMeche added a commit to JeanMeche/angular that referenced this issue Feb 3, 2023
`DebugElement.query()` can return null, let's reflect that on the return type.
This commit also provides a migration to insert non-null assertions after `DebugElement.query()` on existing code bases.

Fixes angular#22449
JeanMeche added a commit to JeanMeche/angular that referenced this issue Feb 3, 2023
`DebugElement.query()` can return null, let's reflect that on the return type.
This commit also provides a migration to insert non-null assertions after `DebugElement.query()` on existing code bases.

Fixes angular#22449
JeanMeche added a commit to JeanMeche/angular that referenced this issue Feb 3, 2023
`DebugElement.query()` can return null, let's reflect that on the return type.
This commit also provides a migration to insert non-null assertions after `DebugElement.query()` on existing code bases.

Fixes angular#22449
JeanMeche added a commit to JeanMeche/angular that referenced this issue Feb 3, 2023
`DebugElement.query()` can return null, let's reflect that on the return type.
This commit also provides a migration to insert non-null assertions after `DebugElement.query()` on existing code bases.

Fixes angular#22449
JeanMeche added a commit to JeanMeche/angular that referenced this issue Feb 3, 2023
`DebugElement.query()` can return null, let's reflect that on the return type.
This commit also provides a migration to insert non-null assertions after `DebugElement.query()` on existing code bases.

Fixes angular#22449
JeanMeche added a commit to JeanMeche/angular that referenced this issue Feb 3, 2023
`DebugElement.query()` can return null, let's reflect that on the return type.
This commit also provides a migration to insert non-null assertions after `DebugElement.query()` on existing code bases.

Fixes angular#22449
JeanMeche added a commit to JeanMeche/angular that referenced this issue Feb 3, 2023
`DebugElement.query()` can return null, let's reflect that on the return type.
This commit also provides a migration to insert non-null assertions after `DebugElement.query()` on existing code bases.

Fixes angular#22449
JeanMeche added a commit to JeanMeche/angular that referenced this issue Feb 3, 2023
`DebugElement.query()` can return null, let's reflect that on the return type.
This commit also provides a migration to insert non-null assertions after `DebugElement.query()` on existing code bases.

Fixes angular#22449
JeanMeche added a commit to JeanMeche/angular that referenced this issue Feb 3, 2023
`DebugElement.query()` can return null, let's reflect that on the return type.
This commit also provides a migration to insert non-null assertions after `DebugElement.query()` on existing code bases.

Fixes angular#22449
JeanMeche added a commit to JeanMeche/angular that referenced this issue Feb 3, 2023
`DebugElement.query()` can return null, let's reflect that on the return type.
This commit also provides a migration to insert non-null assertions after `DebugElement.query()` on existing code bases.

Fixes angular#22449
JeanMeche added a commit to JeanMeche/angular that referenced this issue Feb 3, 2023
`DebugElement.query()` can return null, let's reflect that on the return type.
This commit also provides a migration to insert non-null assertions after `DebugElement.query()` on existing code bases.

Fixes angular#22449
JeanMeche added a commit to JeanMeche/angular that referenced this issue Feb 3, 2023
`DebugElement.query()` can return null, let's reflect that on the return type.
This commit also provides a migration to insert non-null assertions after `DebugElement.query()` on existing code bases.

Fixes angular#22449
JeanMeche added a commit to JeanMeche/angular that referenced this issue Feb 4, 2023
`DebugElement.query()` can return null, let's reflect that on the return type.
This commit also provides a migration to insert non-null assertions after `DebugElement.query()` on existing code bases.

Fixes angular#22449
JeanMeche added a commit to JeanMeche/angular that referenced this issue Mar 31, 2023
`DebugElement.query()` can return null, let's reflect that on the return type.
This commit also provides a migration to insert non-null assertions after `DebugElement.query()` on existing code bases.

Fixes angular#22449
JeanMeche added a commit to JeanMeche/angular that referenced this issue Mar 31, 2023
`DebugElement.query()` can return null, let's reflect that on the return type.
This commit also provides a migration to insert non-null assertions after `DebugElement.query()` on existing code bases.

Fixes angular#22449
JeanMeche added a commit to JeanMeche/angular that referenced this issue Apr 4, 2023
`DebugElement.query()` can return null, let's reflect that on the return type.
This commit also provides a migration to insert non-null assertions after `DebugElement.query()` on existing code bases.

Fixes angular#22449
JeanMeche added a commit to JeanMeche/angular that referenced this issue May 1, 2023
`DebugElement.query()` can return null, let's reflect that on the return type.
This commit also provides a migration to insert non-null assertions after `DebugElement.query()` on existing code bases.

Fixes angular#22449
JeanMeche added a commit to JeanMeche/angular that referenced this issue Aug 11, 2023
`DebugElement.query()` can return null, let's reflect that on the return type.
This commit also provides a migration to insert non-null assertions after `DebugElement.query()` on existing code bases.

Fixes angular#22449
JeanMeche added a commit to JeanMeche/angular that referenced this issue Oct 9, 2023
`DebugElement.query()` can return null, let's reflect that on the return type.
This commit also provides a migration to insert non-null assertions after `DebugElement.query()` on existing code bases.

Fixes angular#22449
@lazarljubenovic
Copy link
Contributor

lazarljubenovic commented Apr 17, 2024

Why does this have only 7 upvotes? Apparently only seven Angular users are writing tests properly 😩 Six years with no response from the team, and there's a pretty damn good PR, with a schematic even! 🥇

Apparently all discussion is moved to the mentioned PR. Hopefully we see this landing in 2024.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: testing Issues related to Angular testing features, such as TestBed cross-cutting: types freq2: medium P4 A relatively minor issue that is not relevant to core functions state: confirmed type: bug/fix
Projects
None yet