-
Notifications
You must be signed in to change notification settings - Fork 25.3k
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
fix(es6): stringify using an opaque ID rather than toString #7825
Conversation
4f35086
to
dfc6c33
Compare
@@ -175,6 +177,15 @@ export function stringify(token): string { | |||
} | |||
|
|||
var res = token.toString(); | |||
if (res.indexOf("(") >= 0) { | |||
let idx = ObjectReference.indexOf(token); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This gets slower over time and contains a memory leak.
Better: Create a facade around a WeakMap
in JS and Expando
in Dart (see https://api.dartlang.org/stable/1.15.0/dart-core/Expando-class.html)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I don't think this is the right place to do this. Rather, we should change our codegen to check for this and keep a local map of the produced symbols. My current PR is already doing this....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you paste the code that breaks? I.e. so that we can find and fix the place where we generate the code, and don't have to do this in this global helper, which might be used for other things than codegen?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved this logic into runtime_metadata.ts
and switched to Map to store the anonymous function objects.
10c0e31
to
d60e838
Compare
@@ -33,6 +33,25 @@ export class RuntimeMetadataResolver { | |||
@Optional() @Inject(PLATFORM_DIRECTIVES) private _platformDirectives: Type[], | |||
@Optional() @Inject(PLATFORM_PIPES) private _platformPipes: Type[]) {} | |||
|
|||
private anonymousTypes = new Map<Object, number>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move up to the other field declarations, before the constructor.
Use _
prefix (so the field is private also in Dart)
…ens. Using toString results in 'function (_arg1, arg2) {' when using closure compiler for 6-to-5.
Merging PR #7825 on behalf of @kara to branch presubmit-kara-pr-7825. |
That's the same fix that's necessary for #6501 |
…ens. Using toString results in 'function (_arg1, arg2) {' when using closure compiler for 6-to-5. Closes angular#7825
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Using toString results in 'function (_arg1, arg2) {' when using closure compiler for 6-to-5.