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

Slowly migrate AMP to a state where we can obfuscate all properties. #9875

Merged
merged 15 commits into from
Jun 14, 2017

Conversation

cramforce
Copy link
Member

@cramforce cramforce commented Jun 13, 2017

Uses the type system to ensure that objects that must be treated as non-obfuscateable are using bracket notation for property access.

This is a first PR of many. Part of #9876

if (opt_initial) {
return /** @type {!JsonObject} */ (opt_initial);
}
return /** @type {!JsonObject} */ ({});
Copy link
Member

Choose a reason for hiding this comment

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

should we do Object.create(null) here

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call. Just made the function pass through to map

*/
this.config_ = /** @type {JSONType} */ ({});
this.config_ = /** @type {JsonObject} */ ({});
Copy link
Member

Choose a reason for hiding this comment

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

would it be better to call dict() here? (besides the implication of adding additional code to the binary)

Copy link
Member Author

Choose a reason for hiding this comment

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

yes

@erwinmombay
Copy link
Member

LGTM

@cramforce cramforce merged commit 016c847 into ampproject:master Jun 14, 2017
@cramforce cramforce deleted the quotes branch June 14, 2017 23:36
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.

3 participants