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

port: Support object comparison in == #3672

Merged
merged 9 commits into from
May 18, 2021
Merged

port: Support object comparison in == #3672

merged 9 commits into from
May 18, 2021

Conversation

Danieladu
Copy link
Contributor

@Danieladu Danieladu commented May 12, 2021

Fixes #3671

Description

Support object comparison in ==

Specific Changes

  1. Support object comparison in contains
  2. Support object comparison for operator == and !=

@Danieladu Danieladu requested review from joshgummersall, stevengum and a team as code owners May 12, 2021 03:37
@coveralls
Copy link

coveralls commented May 12, 2021

Pull Request Test Coverage Report for Build 851669384

  • 24 of 25 (96.0%) changed or added relevant lines in 2 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.01%) to 85.46%

Changes Missing Coverage Covered Lines Changed/Added Lines %
libraries/adaptive-expressions/src/functionUtils.ts 18 19 94.74%
Files with Coverage Reduction New Missed Lines %
libraries/botbuilder-lg/src/templateExtensions.ts 1 84.48%
Totals Coverage Status
Change from base Build 851306174: 0.01%
Covered Lines: 19239
Relevant Lines: 21415

💛 - Coveralls

joshgummersall
joshgummersall previously approved these changes May 13, 2021
@joshgummersall joshgummersall dismissed their stale review May 13, 2021 17:08

accidental

found = args[0].includes(args[1]);
} else if (args[0] instanceof Map) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it seem like this logic is getting lost in this refactor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. The logic of contains of Map would be handlered in accessProperty

*/
private static convertToObj(instance: unknown) {
if (FunctionUtils.getPropertyCount(instance) >= 0) {
if (instance instanceof Map) {
Copy link
Contributor

@joshgummersall joshgummersall May 14, 2021

Choose a reason for hiding this comment

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

Last suggestion, then I promise I won't code-golf anymore on this PR 😄

const entries = instance instanceof Map ? instance.entries() : Object.entries(instance);
return entries.reduce((acc, [key, value]) => ({...acc, [key]: FunctionUtils.convertToObj(value)}), {});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

More suggestions better! Means more can learn.
reduce could not apply on IterableIterator directly. So I use Array.from(instance.entries()) to convert it into Array.

@Danieladu
Copy link
Contributor Author

The browserBot is always failing. But I could not access the details of the error message. @joshgummersall Could you help to take a look and whether some APIs are unavailable.

@joshgummersall
Copy link
Contributor

@Danieladu yep I discovered that myself today! I'll land this and try to resolve it tomorrow.

@joshgummersall joshgummersall merged commit a391ba0 into main May 18, 2021
@joshgummersall joshgummersall deleted the hond/equal branch May 18, 2021 03:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

port: Support object comparison in == (#5575)
3 participants