Skip to content

fix: return 'null' (instead of 'undefined') on lazy relations that have no results#7147

Merged
pleerock merged 1 commit intotypeorm:masterfrom
securedirective:fix-lazy-relation-undefined
Jan 11, 2021
Merged

fix: return 'null' (instead of 'undefined') on lazy relations that have no results#7147
pleerock merged 1 commit intotypeorm:masterfrom
securedirective:fix-lazy-relation-undefined

Conversation

@securedirective
Copy link
Contributor

@securedirective securedirective commented Dec 3, 2020 •
edited
Loading

Description of change

Fix an issue where a lazy relation returns a Promise that resolves to 'undefined'. As implied by
documentation, if the database has a NULL or no results, the result should be a literal 'null'
in Javascript. Also added units tests to hilight the three scenarios in which this occurred.

This PR is to fix reported issue #7146.

Pull-Request Checklist

  • Code is up-to-date with the master branch
  • npm run lint passes with this change
  • npm run test passes with this change
  • This pull request links relevant issues as Fixes #0000
  • There are new or updated unit tests validating the change
  • Documentation has been updated to reflect this change
  • The new commits follow conventions explained in CONTRIBUTING.md

@securedirective securedirective mentioned this pull request Dec 3, 2020
Closed
21 tasks
@securedirective securedirective force-pushed the fix-lazy-relation-undefined branch 4 times, most recently from f22b01d to 53ef708 Compare December 3, 2020 13:24
@securedirective
fix: return 'null' (instead of 'undefined') on lazy relations that ha…
e98f4f0 
…ve no results (typeorm#7146)

Fix an issue where a lazy relation returns a Promise that resolves to 'undefined'. As implied by
documentation, if the database has a NULL or no results, the result should be a literal 'null'
in Javascript. Also added units tests to hilight the three scenarios in which this occurred.

Closes typeorm#7146
@securedirective securedirective force-pushed the fix-lazy-relation-undefined branch from 53ef708 to e98f4f0 Compare December 3, 2020 17:23
@securedirective
Copy link
Contributor Author

securedirective commented Dec 3, 2020

Prior to applying the fix, here's what my unit tests showed:

  github issues > #7146 Lazy relations resolve to 'undefined' instead of 'null'
    lazy-loaded relations
      1) should return null if ManyToOne relation has NULL in database
      2) should return null if OneToOne+JoinColumn relation has NULL in database
      3) should return null if OneToOne relation has NULL in database
    lazy-loaded relations included in 'relations' find option
      ✓ should return null if ManyToOne relation has NULL in database
      ✓ should return null if OneToOne+JoinColumn relation has NULL in database
      ✓ should return null if OneToOne relation has NULL in database
    eager-loaded relations
      ✓ should return null if ManyToOne relation has NULL in database
      ✓ should return null if OneToOne+JoinColumn relation has NULL in database
      ✓ should return null if OneToOne relation has NULL in database


  6 passing (5s)
  3 failing

  1) github issues > #7146 Lazy relations resolve to 'undefined' instead of 'null'
       lazy-loaded relations
         should return null if ManyToOne relation has NULL in database:
     AssertionError: expected undefined to be null
      at /home/kenneth/typeorm/test/github-issues/7146/issue-7146.ts:31:51
      at step (node_modules/tslib/tslib.js:141:27)
      at Object.next (node_modules/tslib/tslib.js:122:57)
      at fulfilled (node_modules/tslib/tslib.js:112:62)
      at processTicksAndRejections (node:internal/process/task_queues:93:5)

  2) github issues > #7146 Lazy relations resolve to 'undefined' instead of 'null'
       lazy-loaded relations
         should return null if OneToOne+JoinColumn relation has NULL in database:
     AssertionError: expected undefined to be null
      at /home/kenneth/typeorm/test/github-issues/7146/issue-7146.ts:37:55
      at step (node_modules/tslib/tslib.js:141:27)
      at Object.next (node_modules/tslib/tslib.js:122:57)
      at fulfilled (node_modules/tslib/tslib.js:112:62)
      at processTicksAndRejections (node:internal/process/task_queues:93:5)

  3) github issues > #7146 Lazy relations resolve to 'undefined' instead of 'null'
       lazy-loaded relations
         should return null if OneToOne relation has NULL in database:
     AssertionError: expected undefined to be null
      at /home/kenneth/typeorm/test/github-issues/7146/issue-7146.ts:43:50
      at step (node_modules/tslib/tslib.js:141:27)
      at Object.next (node_modules/tslib/tslib.js:122:57)
      at fulfilled (node_modules/tslib/tslib.js:112:62)
      at processTicksAndRejections (node:internal/process/task_queues:93:5)

After applying the fix:

  github issues > #7146 Lazy relations resolve to 'undefined' instead of 'null'
    lazy-loaded relations
      ✓ should return null if ManyToOne relation has NULL in database (71ms)
      ✓ should return null if OneToOne+JoinColumn relation has NULL in database
      ✓ should return null if OneToOne relation has NULL in database
    lazy-loaded relations included in 'relations' find option
      ✓ should return null if ManyToOne relation has NULL in database
      ✓ should return null if OneToOne+JoinColumn relation has NULL in database
      ✓ should return null if OneToOne relation has NULL in database
    eager-loaded relations
      ✓ should return null if ManyToOne relation has NULL in database
      ✓ should return null if OneToOne+JoinColumn relation has NULL in database
      ✓ should return null if OneToOne relation has NULL in database


  9 passing (6s)

@securedirective securedirective marked this pull request as ready for review December 3, 2020 19:20
@securedirective securedirective changed the title test: add tests to hilight null/undefined issue with lazy relations (… fix: return 'null' (instead of 'undefined') on lazy relations that have no results (#7146) Dec 5, 2020
@securedirective securedirective changed the title fix: return 'null' (instead of 'undefined') on lazy relations that have no results (#7146) fix: return 'null' (instead of 'undefined') on lazy relations that have no results Dec 5, 2020
@pleerock
Copy link
Member

pleerock commented Jan 11, 2021

Seems reasonable. Thank you for contribution!

@pleerock pleerock merged commit 9b278c9 into typeorm:master Jan 11, 2021
@securedirective
Copy link
Contributor Author

securedirective commented Jan 14, 2021

I think that was the first time I've created a Pull Request on an open-source project, complete with unit tests, and had it accepted. Thank you for the experience!

@securedirective securedirective deleted the fix-lazy-relation-undefined branch January 14, 2021 01:58
This was referenced Mar 15, 2021
Merged
Closed
Closed
Merged
Closed
@pekunicki
Copy link

pekunicki commented Mar 22, 2021

This issue still occurs when you retrieve a newly created item via .save(...) 😞

@gregor-dissemond gregor-dissemond mentioned this pull request Jan 28, 2022
Closed
@gregor-dissemond
Copy link

gregor-dissemond commented Feb 14, 2022

Did this PR introduce the bug #8582?
I am not sure about internals, so this is just a guess.
@securedirective can you elaborate on that?

zshipleyTAG pushed a commit to Amherst-Development/typeorm that referenced this pull request Oct 7, 2022
@zship
Merge branch 'typeorm-0.2.30' into zs/merge-typeorm-0.2.30
56a3821 
* typeorm-0.2.30: (212 commits)
  version bump
  docs: fix javascript usage examples (typeorm#7031)
  fix: resolve migration for UpdateDateColumn without ON UPDATE clause (typeorm#7057)
  fix: Error when sorting by an embedded entity while using join and skip/take (typeorm#7082)
  fix: resolves Postgres sequence identifier length error (typeorm#7115)
  feat: closure table custom naming (typeorm#7120)
  feat: relations: Orphaned row action (typeorm#7105)
  docs: fix invalid code block in "find many options" (typeorm#7268)
  docs: Embodying the example (typeorm#7116)
  docs: document withDeleted option (typeorm#7132)
  fix: return 'null' (instead of 'undefined') on lazy relations that have no results (typeorm#7146) (typeorm#7147)
  docs: update cascade options (typeorm#7140)
  docs: add .ts to supported ormconfig formats (typeorm#7139)
  fix: improve stack traces when using persist executor (typeorm#7218)
  refactor: remove Oracle multirow insert workaround (since typeorm#6927) (typeorm#7083)
  feat: add NOWAIT and SKIP LOCKED lock support for MySQL (typeorm#7236)
  docs: update OneToMany grammar (typeorm#7252)
  feat: JavaScript file migrations output (typeorm#7253)
  docs: update Repository.ts (typeorm#7254)
  chore: update dependency cli-highlight to v2.1.10 (typeorm#7265)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@securedirective @pleerock @pekunicki @gregor-dissemond