mojo: Should we specify response params in generated externs? |
|
Issue description
Currently the generated externs for interfaces only specify "Promise" as the return type. Should they specify the type that will be returned in the promise as well?
From:
/** @interface */
device.mojom.BluetoothSystemInterface = class {
/**
* @return {Promise}
*/
getState() {}
}
To:
/** @interface */
device.mojom.BluetoothSystemInterface = class {
/**
* @return {Promise<device.mojom.BluetoothSystem_GetState_ResponseParams>}
*/
getState() {}
}
device.mojom.BluetoothSystem_GetState_ResponseParams is defined as:
device.mojom.BluetoothSystem_GetState_ResponseParams = class {
constructor() {
/** @type { !device.mojom.BluetoothSystem.State } */
this.state;
}
};
,
Dec 11
Sure, but that wouldn't be accurate right now because the generated symbols for structs do not actually describe the structure in a way that is meaningful to consumers. I discussed that problem with calamity@ yesterday and we concluded that we should change this so that: - the symbols we generate for structs now should have some kind of mangled name/namespace, since they are only used internally by bindings code - we should annotate the symbols we generate now as closure record types, since that accurately reflects how they're used Of course this has the same issue as the @interfaces we used to generate for each mojom interface, namely that the definitions take up space without having any actual use for non-compiled JS, other than developer reference. Still, seems like a good idea to generate them if we can at least minimize/comment-strip the non-compiled JS.
,
Dec 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/94cbc94bffae9985b93e9b8381907726faf5efb7 commit 94cbc94bffae9985b93e9b8381907726faf5efb7 Author: Christopher Lam <calamity@chromium.org> Date: Wed Dec 19 06:53:53 2018 [Mojo Lite] Use record types for externs. This CL changes Mojo Lite Closure Externs to use record types which more closely reflects the reality of Mojo Lite bindings (that there is no constructor for a generated class). This will allow Object literals that implement the record type interface to assume the type of a generated Mojo Lite struct type. It also fixes up some nullability issues with Arrays and Maps. Bug: 849993, 914149 Change-Id: I98d69689d4a0ffb6b773cef61041130a1934b1f5 Reviewed-on: https://chromium-review.googlesource.com/c/1372971 Commit-Queue: calamity <calamity@chromium.org> Reviewed-by: Ken Rockot <rockot@google.com> Cr-Commit-Position: refs/heads/master@{#617745} [modify] https://crrev.com/94cbc94bffae9985b93e9b8381907726faf5efb7/chrome/browser/resources/app_management/fake_page_handler.js [modify] https://crrev.com/94cbc94bffae9985b93e9b8381907726faf5efb7/chrome/browser/resources/omnibox/omnibox_output.js [modify] https://crrev.com/94cbc94bffae9985b93e9b8381907726faf5efb7/mojo/public/tools/bindings/generators/js_templates/lite/struct_externs.tmpl [modify] https://crrev.com/94cbc94bffae9985b93e9b8381907726faf5efb7/mojo/public/tools/bindings/generators/mojom_js_generator.py |
|
►
Sign in to add a comment |
|
Comment 1 by ortuno@chromium.org
, Dec 11