Make JS value serialization more flexible #385
Conversation
This is required to allow one-shot serialisation of nested objects, which is important to avoid having to make multiple script calls to get the data from nested objects.
jgraham
force-pushed
the
script_max_depth
branch
from
5f69d6e
to
732035b
Compare
There was a problem hiding this comment.
Just a single question. Otherwise this is a great idea.
@sadym-chromium can you please review as well?
index.bs
Outdated
| awaitPromise: bool, | ||
| target: script.Target, | ||
| ?arguments: [*script.ArgumentValue], | ||
| ?maxDepth: js-uint, |
There was a problem hiding this comment.
I assume that as long it is not clear if we can use defaults here you wont further use them? Otherwise we could set a default of 1 already here and won't need the prose.
|
UPD: Moved to the review
|
There was a problem hiding this comment.
In the light of #390 adding this parameter without changing the serialization logic is pushing users to make the potentially fatal mistake with using big-enough maxDepth by default.
|
I think "potentially fatal" is wildly overstating the potential impact here. But sure, I'm happy to change this PR to add more finegrained control over serialization. |
jgraham
changed the title
Instead of having a `maxDepth` field, have a `serializationOptions` field that allows specifying multiple constraints on what's serialized. The initial supported constraints are: `maxObjectDepth`: The depth to which to serialize plain js objects (i.e. not DOM nodes). Defaults to null i.e. unlimited depth (matching WebDriver classic). `maxDomDepth`: Depth to which to serialize DOM nodes. Defaults to 0 i.e. serialize a node without any children. `includeShadowTree`: Whether to descend into shadow trees when serializing DOM nodes. Values are "none", meaning don't serialize shdow trees, "open" meaning only serialize open shadow trees, and "all" meaning serialize any shadow tree. Serialization of the tree itself follows the `maxDomDepth` parameter (but in all cases where an element has a shadow root, the shadow root itself is serialized).
jgraham
force-pushed
the
script_max_depth
branch
from
e69649e
to
5d387bb
Compare
|
LGTM |
Co-authored-by: Henrik Skupin <mail@hskupin.info>
There was a problem hiding this comment.
Looks fine to me. Thanks!
Regarding tests we can update those as part of the work on bug https://bugzilla.mozilla.org/show_bug.cgi?id=1824953.
SHA: 99db2d4 Reason: push, by jgraham Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This is required to allow one-shot serialisation of nested objects, which is important to avoid having to make multiple script calls to get the data from nested objects.
Preview | Diff