wintercg / proposal-common-minimum-api

https://common-min-api.proposal.wintercg.org/
Other
227 stars 13 forks source link

How does the `FormData` constructor handle arguments in non-DOM runtimes? #63

Open andreubotella opened 9 months ago

andreubotella commented 9 months ago

The FormData constructor, as defined in the XHR spec, takes two optional parameters: the first of type HTMLFormElement, and the second of type HTMLElement. These are DOM APIs, which server-side runtimes don't support.

The way I see it, if any of these arguments is passed (and it's anything other than undefined), this indicates a bug in user code. For example, the code in question could be isomorphic code using DOM APIs through a library such as jsdom or deno-dom, and the developer mistakenly used those objects with the runtime's FormData implementation. TypeScript might not catch that, since it could be set up to use the browser's type definitions rather than the runtime's.

In the runtimes I've tested:

No runtimes seem to check the second argument. Specifying Node.js and Deno's behavior might be fine, since in browsers the second argument does nothing if the first one is undefined, and the first argument is already checked. But the WebIDL implementation in browsers does check that the second argument is a valid HTMLElement regardless of whether the first one is undefined, so maybe we should be consistent with that. This would also let us specify this by just saying what happens with unsupported WebIDL types, which is something that might also apply to other APIs in the future.

ekwoka commented 9 months ago

Should they have an error in the event of a DOM environment being used though? Assuming the HTMLFormElement is implemented to spec?

Shouldn't the runtimes at least check if it is a valid form?

andreubotella commented 9 months ago

Should they have an error in the event of a DOM environment being used though? Assuming the HTMLFormElement is implemented to spec?

Shouldn't the runtimes at least check if it is a valid form?

If the runtime doesn't implement the DOM, it shouldn't need to be aware of which possible libraries running on that runtime might implement a shim for the DOM API. How should Node.js be able to tell that a jsdom HTMLFormElement is indeed an HTMLFormElement that represents a valid HTML form in any way, and how is it supposed to know how to extract the form list from it? If jsdom wants to make that work, it's up to them to monkeypatch the built-in FormData.

ekwoka commented 8 months ago

How should Node.js be able to tell that a jsdom HTMLFormElement is indeed an HTMLFormElement that represents a valid HTML form in any way

It doesn't need to.

It just assumes it is, since there is nothing else it could be.

It does not need to check, it can just see there is SOMETHING and start accessing it like an HTMLFormElement.

So basically, EXACTLY the same way Chrome or Safari would start working with whatever you pass in.

how is it supposed to know how to extract the form list from it?

HTMLFormElement is spec'd. How the FormData constructor access the form data is spec'd. https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#constructing-the-form-data-set

They need no special knowledge.

If jsdom wants to make that work, it's up to them to monkeypatch the built-in FormData.

Isn't it up to the runtime to be spec compliant? Not libraries to make the runtime spec compliant?

While this is on the edge (between where server sensible APIs and browser only APIs meet), it seems the most appropriate for the runtimes to treat the passed object as if it is an HTMLFormElement, and TypeError if it ends up not being compliant.

The runtime doesn't need special knowledge or functioning DOM to be able to get the entries from a form element-like object.

andreubotella commented 8 months ago

If you pass a random object as the first argument to the FormData constructor in browsers, they will not assume that that must be an HTMLFormElement object, "since there is nothing else it could be". They will check that it is, and throw otherwise. Also, the "constructing the entry list" algorithm that the FormData constructor calls in browsers relies on internal state of various form DOM APIs that isn't exposed through the JS API, such as the submission value of a form-associated custom element (since there's no way to access the ElementInternals object). Etc...

ekwoka commented 8 months ago

It doesn't not need any of that.

It can construct the form data from the publicly exposed interface.

https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#constructing-the-form-data-set

This isn't difficult to implement without accessing internals.

Yeah, it could be polyfilled, but I think, when it's spec'd, it is just smarter for the runtime to implement than to encourage libraries to monkey patch the runtime.

andreubotella commented 8 months ago

It doesn't not need any of that.

It can construct the form data from the publicly exposed interface.

https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#constructing-the-form-data-set

This isn't difficult to implement without accessing internals.

Yeah, it could be polyfilled, but I think, when it's spec'd, it is just smarter for the runtime to implement than to encourage libraries to monkey patch the runtime.

Form-associated custom elements can be form controls, and their contribution to the form entry list is given by their entry construction algorithm (see step 5.4 in the "constructing the entry list" algorithm), which in turn relies on their submission value, which is not publicly exposed through their DOM interface. It's only accessible through the ElementInternals object, which you can only get by calling the element's attachInternals() method, and that throws if you call it more than once. Browsers have to rely on internal state for this.

I know this because I was involved in specifying some things in the "construct the entry list" algorithm a few years ago, and I had to think about some interactions with form-associated custom elements and write WPT tests for that.

But aside from all of that, I don't expect any server-side runtime would be willing to implement this.

ekwoka commented 8 months ago

I think it could also be fair to not do any implementations that are impossible, just the simplest.

On a side note, pretty bad for them to make these things internal in the first place...