-
Notifications
You must be signed in to change notification settings - Fork 38
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
Feature: Chrome APIs #120
Feature: Chrome APIs #120
Conversation
This changeset introduces initial support for `chrome` APIs, as requested in #119. Externs added from Closure Compiler: - `contrib/externs/chrome.js`
@@ -54,6 +54,7 @@ Following are the different elemental2 modules and their target names: | |||
webgl | `@com_google_elemental2//:elemental2-webgl-j2cl` | |||
media | `@com_google_elemental2//:elemental2-media-j2cl` | |||
webstorage | `@com_google_elemental2//:elemental2-webstorage-j2cl` | |||
chrome | `@com_google_elemental2//:elemental2-chrome-j2cl` |
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.
standard README
updates
# Chrome api | ||
jsinterop_generator( | ||
name = "elemental2-chrome", | ||
exports = ["//java/elemental2/chrome"], |
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.
top-level target for elemental2-chrome
--- | ||
> * @extends {ChromeBaseEvent<!Function>} | ||
335c335 | ||
< * @extends {ChromeBaseEvent<function(!Array<string>)>} |
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.
the build was having issues with these signatures, so i've made them generic for now, until they can get better type visibility
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.
You should try to fix that in https://github.com/google/closure-compiler/blob/master/contrib/externs/chrome.js directly.
patch_extern_file( | ||
name = "chrome_patched", | ||
src = "//third_party:chrome.js", | ||
patch_file = "chrome.js.diff", |
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.
simple patch is applied via elemental2
's patching defs
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.
Note I'm working on elemental now in order to get rif of these patch files. This is a burden in term of extern maintenance and require a continuous sync between elemental2 and closure github repo.
elemental2.chrome.Tab.windowId | ||
elemental2.chrome.Tab.openerTabId | ||
elemental2.chrome.Tab.width | ||
elemental2.chrome.Tab.height |
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.
these are all the simple integer-seeming types i could find in the chrome
extern namespace, at first glance
|
||
alias( | ||
name = "chrome_extensions.js", | ||
actual = "@com_google_closure_compiler//:contrib/externs/chrome_extensions.js", |
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.
and, finally, update to third_party/BUILD
to bring in the relevant extern files.
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.
I think that this should be an third party library and not part of elemental2.
Elemental2 aims to provide standardized javascript native api and this is specific to chrome.
So you should create your own github project. I'll be glad to help you to review the code if needed.
patch_extern_file( | ||
name = "chrome_patched", | ||
src = "//third_party:chrome.js", | ||
patch_file = "chrome.js.diff", |
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.
Note I'm working on elemental now in order to get rif of these patch files. This is a burden in term of extern maintenance and require a continuous sync between elemental2 and closure github repo.
380,382c380,382 | ||
< USER: '', | ||
< CAPTURE: '', | ||
< EXTENSION: '', |
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.
useless diff
--- | ||
> * @extends {ChromeBaseEvent<!Function>} | ||
335c335 | ||
< * @extends {ChromeBaseEvent<function(!Array<string>)>} |
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.
You should try to fix that in https://github.com/google/closure-compiler/blob/master/contrib/externs/chrome.js directly.
@jDramaix, understood. Thank you for your response |
This changeset introduces initial support for
chrome
APIs, as requested in #119. I'm filing it early to get feedback and make sure my approach is solid. I don't know if you guys have this on your roadmap or not. No worries if this PR is out of place temporally or the approach is entirely wrong.Externs added from Closure Compiler:
contrib/externs/chrome.js
Progress:
chrome.js
chrome_extensions.js