Skip to content

Conversation

@bajtos
Copy link
Member

@bajtos bajtos commented Jun 14, 2019

The testing framework Jest creates a new V8 VM for each test file and as a result, it's not possible to rely on instanceof Function checks to work - there may be multiple Function constructors present.

This commit reworks instanceof Function check to use typeof instead.

See also #3159

Checklist

👉 Read and sign the CLA (Contributor License Agreement) 👈

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

👉 Check out how to submit a PR 👈

The testing framework Jest creates a new V8 VM for each test file
and as a result, it's not possible to rely on `instanceof Function`
checks to work - there may be multiple `Function` constructors
present.

This commit reworks `instanceof Function` check to use `typeof` instead.

Signed-off-by: Miroslav Bajtoš <mbajtoss@gmail.com>
@bajtos bajtos requested review from a team and raymondfeng June 14, 2019 11:45
@bajtos bajtos self-assigned this Jun 14, 2019
@bajtos
Copy link
Member Author

bajtos commented Jun 14, 2019

I am leaving out testing this change because I don't fully understand how is Jest creating the situation where there is more than one Function constructor present and I suspect that recreating this situation in Mocha tests would be too cumbersome.

@bajtos bajtos added bug IoC/Context @loopback/context: Dependency Injection, Inversion of Control labels Jun 14, 2019
@bajtos bajtos merged commit a009aaf into master Jun 14, 2019
@bajtos bajtos deleted the fix/metadata-function-check branch June 14, 2019 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug IoC/Context @loopback/context: Dependency Injection, Inversion of Control

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants