ampproject / ampproject/amphtml

Eliminate the need to cast from `Object` to `JsonObject`

Open
#23,567 14 comments 0 reactions 2 assignees Claimed by @erwinmombay View on GitHub
Stale Type: Bug WG: performance
Dominant language
JavaScript
Stars
14.9k
Forks
4.1k
PR merge metrics
No merged PRs in 30d

Description

This came out of the discussions at https://github.com/ampproject/amphtml/pull/23559#discussion_r308034580 and https://github.com/ampproject/amphtml/pull/23559#discussion_r308034854

The AMP codebase contains [hundreds of casts](https://github.com/ampproject/amphtml/search?q=%22%40type+%7B%21JsonObject%7D%22&unscoped_q=%22%40type+%7B%21JsonObject%7D%22) from `Object` to `JsonObject`.

- I wonder if there's a way to modify the declaration of `JsonObject` in `amp.extern.js` to make most of the casts unnecessary
https://github.com/ampproject/amphtml/blob/6b86000a91ccd59023518db7437fc28f7579a5cb/build-system/amp.extern.js#L57-L65

- For converting to `JsonObject`, there is `parseJson()` from `src/json.js`, but it doesn't work for cases where a non-null `JsonObject` is expected
https://github.com/ampproject/amphtml/blob/c4a663d0ba13d0488c6fe73c55dc8c971ac6ec0d/src/json.js#L104-L113

- For checking if a variable is an `Object`, there is `isObject()` from `src/types.js`, but it doesn't work when a `JsonObject` is expected
https://github.com/ampproject/amphtml/blob/c4a663d0ba13d0488c6fe73c55dc8c971ac6ec0d/src/types.js#L48-L55

Related to #23425

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.