Skip to content

Conversation

@srujzs
Copy link
Contributor

@srujzs srujzs commented Jul 21, 2021

The debugger filters out native JS objects using the classRef's name. Since DDC will change this representation, we need to check for the new representation.

Checked that Flutter run for web shows no native javascript objects in static scope succeeds with and without https://dart-review.googlesource.com/c/sdk/+/199600.

The debugger filters out native JS objects using the classRef's
name. Since DDC will change this representation, we need to
check for the new representation.
@srujzs srujzs requested a review from annagrin July 21, 2021 18:10
@srujzs srujzs self-assigned this Jul 21, 2021
@google-cla google-cla bot added the cla: yes label Jul 21, 2021
(instanceRef?.classRef?.name == 'JavaScriptObject' &&
instanceRef?.classRef?.library?.uri == 'dart:_interceptors') ||
// Old type representation still needed to support older SDK versions.
instanceRef?.classRef?.name == 'NativeJavaScriptObject';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that we don't have a library for the previous type representation (NativeJavaScriptObject) since it's part of the DDC runtime, so I left it as is.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah the library in the old representation showed as "null" :)

Copy link
Contributor

@annagrin annagrin left a comment

Choose a reason for hiding this comment

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

LGTM!

(instanceRef?.classRef?.name == 'JavaScriptObject' &&
instanceRef?.classRef?.library?.uri == 'dart:_interceptors') ||
// Old type representation still needed to support older SDK versions.
instanceRef?.classRef?.name == 'NativeJavaScriptObject';
Copy link
Contributor

Choose a reason for hiding this comment

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

yeah the library in the old representation showed as "null" :)

@srujzs srujzs merged commit 55308d2 into dart-lang:master Jul 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants