microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
101.33k stars 12.54k forks source link

allow constructing URLSearchParams directly from FormData #30584

Closed thatbudakguy closed 3 years ago

thatbudakguy commented 5 years ago

TypeScript Version: 3.4.0-dev.20190323

Search Terms: URLSearchParams, FormData

Code

let form = document.createElement('form')
console.log(new URLSearchParams(new FormData(form)))

Expected behavior: compilation succeeds and an (empty) URLSearchParams instance is constructed using the provided FormData. The above code runs without issue on Chrome 73 and Firefox 67 for me.

Actual behavior: compilation fails because URLSearchParams doesn't explicitly allow an argument of type FormData as input to the constructor.

Argument of type 'FormData' is not assignable to parameter of type 'string | Record<string, string> | URLSearchParams | string[][]'.
  Property 'sort' is missing in type 'FormData' but required in type 'URLSearchParams'.

Playground Link: link))

Related Issues:

12517

15338

thatbudakguy commented 5 years ago

If all that's required for this is to add | FormData to the constructor signature, I'm happy to PR.

saschanaz commented 5 years ago

19806 covers this.

leonstafford commented 5 years ago

@thatbudakguy - in the meantime, do you have a workaround for this?

thatbudakguy commented 5 years ago

@leonstafford my current workaround is just // @ts-ignore, sadly.

raythurnvoid commented 5 years ago

Another workaround is: new URLSearchParams(new FormData(event.currentTarget) as any)

JirkaDellOro commented 5 years ago

Is there a deadline for this bug to be fixed?

JirkaDellOro commented 4 years ago

No? Yes? When?

Spongman commented 4 years ago

any news?

PaperStrike commented 3 years ago

I don't think it will be "fixed" as FormData may contain File while URLSearchParams only accepts String.

That code runs successfully in browsers by the implicit conversions in JS. Explicit matched types are needed in TS. Try to filter out the Files, or convert them to Strings. For example,

// For files, use their names
const convertedFormEntries = Array.from(
  new FormData(),
  ([key, value]) => (
    [key, typeof value === 'string' ? value : value.name]
  ),
);
const searchParams = new URLSearchParams(convertedFormEntries);

You can see the error more clearly in this TS playground.

JirkaDellOro commented 3 years ago

Well, MDN writes that the constructor accepts one the following:

Since FormData is explicitly mentioned, TypeScript should gladly compile it and the definition file for the dom should include the mentioned types in the constructors signature. The fact that file won't show a filename but [object File] is a logical problem at runtime, not at compiletime.

https://jsfiddle.net/3ng6qkL1/

saschanaz commented 3 years ago

The fact that file won't show a filename but [object File] is a logical problem at runtime, not at compiletime.

And TypeScript is all for preventing such logical typing problems.

Maybe FormData could be made generic so that this can work:

const data = new FormData<string>();
console.log(new URLSearchParams(data));

I'm not sure new FormData<string>(formElement) makes sense since the element may include non-strings.

JirkaDellOro commented 3 years ago

And TypeScript is all for preventing such logical typing problems.

True, On the other hand, TypeScript should not prevent standard functionality to work as intended.

PaperStrike commented 3 years ago

MDN isn't the standard, strictly speaking.

The file name conversion is defined in form entry list to name-value pairs conversion, HTML Standard. Click on the second bold sentence, it shows where it is used.

The note about [object File] isn't mentioned anywhere in steps to initialize a URLSearchParams, URL Standard. It happens by implicit conversions.

JirkaDellOro commented 3 years ago

Now let me perform an elegant 180 after digging into it. This issue should not be labeled bug but works as intended and praised as a feature of TypeScript.

thatbudakguy commented 3 years ago

closing, since as others have mentioned it's not a bug for TypeScript to avoid coercing File despite this being common behavior elsewhere. thread offers several workarounds for anyone still looking to do this.

sacru2red commented 1 year ago

Negated types feature could clear it. in future any not File

or FormData methods should take a Generic #43797

lightyaer commented 11 months ago

Another workaround:


  const formData = new FormData();
  formData.set('something', "something");

  const searchParams = new URLSearchParams(
    formData as unknown as Record<string, string>,
  ).toString();

if value is an object, then JSON.stringify(object);

thoughtsunificator commented 1 month ago

By reading the FormData spec :

Image

You can see that FormData is an iterable whose values are a sequence that contains a both USVString and FormDataEntryValue (which itself can be a File or USVString) as values, and according to the spec:

Objects implementing an interface that is declared to be iterable support being iterated over to obtain a sequence of values.

FormData is an iterable according to the same spec.

And File is implementing Serializable, so for the serialization part, it says that :

The resulting data serialized into serialized must be independent of any realm.

The spec also says that URLSearchParams is an iterable<USVString, USVString>, it expects a sequence of two strings.

It would then make sense for URLSearchParams to try to serialize what can be, it's the whole point of serialization in the first place.

Hence, the [object File].

An Error, being a Serializable, will also be serialized:

const form = document.createElement('form')
console.log(new URLSearchParams([[new Error("foo"),"bar"]]).toString())
// Output "Error%3A+foo=bar"

Which means, that FormData is a valid init parameter according to the URLSearchParams IDL:

Image

ITenthusiasm commented 1 month ago

@thatbudakguy any thoughts about the above? If you agree, would you up for re-opening this issue?

PaperStrike commented 1 month ago

@thoughtsunificator

  1. Number, undefined, null, File, FileList, Error, the array and object combinations of all these and many more are serializable. You don't need TypeScript if you consider them all as strings, just type in @ts-expect-error.
  2. For objects annotated with [Serializable] attribute specifically, they have detailed desired serialization and deserialization steps that should be taken and let's take File as an example

    File objects are serializable objects. Their serialization steps, given value and serialized, are:

    1. Set serialized.[[SnapshotState]] to value’s snapshot state.
    2. Set serialized.[[ByteSequence]] to value’s underlying byte sequence.
    3. Set serialized.[[Name]] to the value of value’s name attribute.
    4. Set serialized.[[LastModified]] to the value of value’s lastModified attribute.

    I don't see how this will make [object File].

thatbudakguy commented 1 month ago

@thatbudakguy any thoughts about the above? If you agree, would you up for re-opening this issue?

I'm not convinced this needs to be reopened. When I opened it, I just wanted to quickly convert a form's data into query parameters, but had forgotten that forms can include File. That seems like exactly the situation where TS is helpful: to notify you of edge cases you might otherwise miss.

If you're in the same situation, use the solution in https://github.com/microsoft/TypeScript/issues/30584#issuecomment-890515551 with a comment about handling File, even if your app will never actually receive a File. Maybe it'll help someone else who comes after you.

ITenthusiasm commented 1 month ago

That seems like exactly the situation where TS is helpful: to notify you of edge cases you might otherwise miss.

I'm not sure that this is true. There are some instances where TypeScript really should not intervene because it starts overstepping by trying to predict runtime behaviors which are intrinsically unpredictable. This is why, for example, JSON.parse() returns any instead of unknown. The unknown type is technically more "type safe", but it is far less convenient or reasonable. And the real bugs resulting from JSON.parse() would be better caught during runtime and/or with tests.

Similarly, regarding FormData, this is a runtime issue. Problems with FormData likely won't be caught with any kind of type system. They'll be caught when submitting data to the server and seeing the response. So I still think it's worth re-opening this issue.

If you're in the same situation, use the solution in https://github.com/microsoft/TypeScript/issues/30584#issuecomment-890515551

The comment that you linked to has negative impacts on performance by creating throw-away memory. Moreover, although I haven't tested it yet, that code might produce bugs for scenarios where a given key has multiple values (e.g., <select multiple>). (I know, for instance, that Object.fromEntries(formData) definitely produces bugs in that scenario.) TypeScript is supposed to be something helpful, but developers shouldn't feel burdened to complicate or slow-down their code simply for a compiler. In a sense, that defeats the point of compilers.

Is there a better solution that you would propose (besides any or @ts-expect-error)? If not, then -- again -- it seems this issue isn't truly completed and should be re-opened, no?

thatbudakguy commented 1 month ago

https://github.com/microsoft/TypeScript/issues/43797 seems like a wholly valid solution.

ITenthusiasm commented 1 month ago

Seems limited but I'll investigate more thoroughly

PaperStrike commented 1 month ago

There are some instances where TypeScript really should not intervene because it starts overstepping by trying to predict runtime behaviors which are intrinsically unpredictable.

Yes, that's why TypeScript has as, ! (the one to exclude nullability), generics, overloads, @ts-ignore, @ts-expect-error and of course any for some totally unpredictables.

This is why, for example, JSON.parse() returns any instead of unknown.

I doubt if TypeScript would pick any if they have unknown since v1.0. Search JSON.parse unknown in this repo and there's so many asking making it unknown. A quick glance over the maintainers' responses look like they relate this change to some bigger change and consider the big change too breaking so they still left it under consideration.

Problems with FormData likely won't be caught with any kind of type system.

TypeScript caught mine and maybe also Budak's problem. I was writing a library that handles forms with possible File fields and confused about the error as you know URLSearchParams and FormData look so perfectly matched.

Moreover, although I haven't tested it yet, that code might produce bugs for scenarios where a given key has multiple values (e.g., <select multiple>).

It won't, just give it a try, it works fine.

TypeScript is supposed to be something helpful, but developers shouldn't feel burdened to complicate or slow-down their code simply for a compiler. In a sense, that defeats the point of compilers.

I agree that it's hard to relate with File in the first place by the error message (maybe caused by https://github.com/microsoft/TypeScript-DOM-lib-generator/issues/741 that the current constructor don't allow string sequence iterables too). TypeScript should somehow improve that but it would be in different issue(s).

For the slow down I used to choose as (as unknown as string[][]) or @ts-expect-error randomly whenever there's no possible File and leave a comment why the cast / expect is there. Recently I rarely deal with FormData directly and I would wrap it to a universal utility that ought to handle any FormData 2 URLSearchParams conversion well if I need to.

Is there a better solution that you would propose (besides any or @ts-expect-error)? If not, then -- again -- it seems this issue isn't truly completed and should be re-opened, no?

as, #43797 generic.

ITenthusiasm commented 1 month ago

Yes, that's why TypeScript has as, ! (the one to exclude nullability), generics, overloads, @ts-ignore, @ts-expect-error and of course any for some totally unpredictables.

Those utilities do solve a number of problems. But I'm not sure I'd say they're intended for the concern that I raised earlier.

I doubt if TypeScript would pick any if they have unknown since v1.0.

I'm basing this on a convo with orta after axios caught flack for changing any to unknown. A mind change may have happened since that incident, but I don't know that unknown is the obvious solution; not everyone wants it.

TypeScript caught mine and maybe also Budak's problem.

That does not mean all problems can be caught (e.g., explicit application/x-www-form-urlencoded w/ [type=file] fields). There's a decent set that still has to be caught at runtime (or by foreknowledge). And to be fair, it's worth questioning whether TS really caught the problem or forced you to research/pursue an alternative.

It won't, just give it a try, it works fine.

You are right. 👍🏾 This was an invalid assumption. Unfortunately, there's still the performance aspect.

For the slow down I used to choose as (as unknown as string[][]) or @ts-expect-error randomly whenever there's no possible File and leave a comment why the cast / expect is there.

It's this dancing that I don't particularly favor (when it's unnecessary). I understand that you do not frequently use the FormData object directly anymore, but that doesn't mean other developers don't.

as, https://github.com/microsoft/TypeScript/issues/43797 generic.

My any comment was referencing type casting in general. (as any is faster than as unknown as *, and they're both effectively the same hack.) That other GH issue was referenced earlier, yes.

sandersn commented 1 month ago

I'm not convinced this is worth re-opening. The spec snippets that @thoughtsunificator posted make it clear that Typescript should regard URLSearchParams's constructor as taking Record<string, string> and FormData as Record<string, string | File> . And Record<string, string | File> isn't assignable to Record<string, string>`.