feat(ext/geometry): Geometry Interfaces Module Level 1 #22054
Conversation
petamoriken
force-pushed
the
feat/geometry
branch
3 times, most recently
from
3343fa4
to
4206e6c
Compare
petamoriken
force-pushed
the
feat/geometry
branch
3 times, most recently
from
b53a1a1
to
7d33171
Compare
| "css": { | ||
| "geometry": { | ||
| "DOMMatrix-001.html": [ | ||
| "new DOMMatrix(\"none\")", |
There was a problem hiding this comment.
These are failing because we don't support using CSS transform strings?
There was a problem hiding this comment.
Yes, currently only unitless matrix and matrix3d functions are supported
| "DOMPointReadOnly interface: existence and properties of interface object", | ||
| "DOMPoint interface: existence and properties of interface object", | ||
| "DOMRectReadOnly interface: existence and properties of interface object", | ||
| "DOMRect interface: existence and properties of interface object", |
There was a problem hiding this comment.
Why is this failing? It shouldn't fail if we implement DOMRect.
There was a problem hiding this comment.
These tests fail because Web IDL requires the interface objects to be defined as data properties but here they are implemented as getters to support lazy loading.
|
@petamoriken could you fix the conflicts? |
|
@petamoriken sorry, but I'm gonna bump this one to Deno v1.43. I discussed this PR with @crowlKats and @littledivy and I have some reservations seeing the amount of JS code this contains. I know it's lazy loaded and shouldn't impact startup time, but we believe this is a good opportunity to finally get the ball rolling and try this API to be implemented using more native code - in particular a lot of work in the APIs is a combination of WebIDL validation and calling an op. We do know from the benchmarks that WebIDL is rather slow and we devised a tentative plan how we can start moving WebIDL validation to Rust code. I think it would be beneficial if we could try to integrate this approach in your PR and its surface area is rather significant and should help us establish if making WebIDL part of ops is feasible. I will open an issue about it soon with some explanations how we want to tackle that and we will get ~10 validators ported to Rust (we'll try to target the validators you are using in this PR). Thank you for the hard work on this and we'll make sure it doesn't rot and gets landed. |
tests/integration/lsp_tests.rs
Outdated
| @@ -5076,7 +5076,7 @@ fn lsp_jsr_auto_import_completion() { | |||
| json!({ "triggerKind": 1 }), | |||
| ); | |||
| assert!(!list.is_incomplete); | |||
| assert_eq!(list.items.len(), 261); | |||
| assert_eq!(list.items.len(), 268); | |||
There was a problem hiding this comment.
Yikes, this is a bit concerning that adding new globals requires changes in seemingly unrelated tests for the LSP. I've opened #23102 to tackle that.
There was a problem hiding this comment.
General question: why do we need both WPT and unit tests for these APIs? Are WPT tests not thorough enough?
There was a problem hiding this comment.
Yes. One example is that WPT doesn't have tests for mutable transform functions such as multiplySelf.
| name = "deno_geometry" | ||
| version = "0.1.0" | ||
| authors.workspace = true |
There was a problem hiding this comment.
Note to self: I already claimed https://crates.io/crates/deno_geometry to avoid problems during publishing when this PR lands
|
@petamoriken I spoke with @littledivy earlier today and he's close to landing all necessary changes to |
closes #21608