Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add initial formData polyfill for Request #361

Merged
merged 3 commits into from Nov 9, 2021

Conversation

jacob-ebey
Copy link
Member

No description provided.

@tchak
Copy link

tchak commented Nov 3, 2021

I think FormData is supposed to implement iterator interface

@ryanflorence
Copy link
Member

I think FormData is supposed to implement iterator interface

Good catch @tchak

let formData = new FormData();
formData.add("single", "heyo");
formData.append("multi", "one");
formData.append("multi", "two");

let results = []
for (let [k, v] of formData) results.push([k,v]);
expect(results).toEqual([
  ["single", "heyo"],
  ["multi", "one"],
  ["multi", "two"]
]);

@jacob-ebey let's add a little unit test on this too and hit each method along with that test ^

@ryanflorence
Copy link
Member

ryanflorence commented Nov 4, 2021

I think the syntax is something weird like this?

class RemixFormData {
  [Symbol.iterator]() {
    return {
      next: () => {
        // when you're done
        return { done: true };
        // when you're still iterating
        return { value };
      }
    }
  }
}

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Iteration_protocols

@mjackson
Copy link
Member

mjackson commented Nov 4, 2021

You already have an iterable object in this._params, so I think you can just do this:

class RemixFormData {
  [Symbol.iterator]() {
    return this._params.entries();
  }
}

@sergiodxa
Copy link
Member

sergiodxa commented Nov 4, 2021

Wouldn't this change make the Remix Request object not be the same as the standard Request object? What if instead of doing request.formData() to get the FormData you pass the instance on the loader arguments?

export let loader: LoaderFunction = async ({ request, formData }) => {
  // use request with the same standard API as documented in MDN
  // and also use formData which should follow the interface also documented in MDN
};

@jacob-ebey
Copy link
Member Author

Wouldn't this change make the Remix Request object not be the same as the standard Request object? What if instead of doing request.formData() to get the FormData you pass the instance on the loader arguments?

export let loader: LoaderFunction = async ({ request, formData }) => {
  // use request with the same standard API as documented in MDN
  // and also use formData which should follow the interface also documented in MDN
};

.formData exists on a fetch Request object: https://developer.mozilla.org/en-US/docs/Web/API/Request/formData

let req = new Request("http://test.com", {
  method: "POST",
  body: "test=value",
  headers: { "Content-Type": "application/x-www-form-urlencoded" },
});
req.formData().then((data) => {
  console.log(data.get("test"));
});

@sergiodxa
Copy link
Member

TIL, ignore me then

@kiliman
Copy link
Collaborator

kiliman commented Nov 5, 2021

TIL, ignore me then

Don't feel bad. I didn't know about that until I saw the example that Ryan posted on the new Remix website.

I've always been doing:

const data = new URLSearchParams(await request.text())

@sergiodxa
Copy link
Member

@kiliman don't worry, I didn't feel bad 🙂 , I was also doing that, I even created a whole utility on Remix Utils for that await bodyParser.toSearchParams(request), once this ships it will become deprecated, this is simpler.

@@ -1,7 +1,7 @@
{
"exclude": ["__tests__"],
"compilerOptions": {
"lib": ["ES2019"],
"lib": ["ES2019", "DOM.Iterable"],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been trying to keep the DOM types out of our node packages! Do you need this for IterableIterator? Or something else?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, "FormData" type isn't itterable without it :/

formData.append("multi", "two");

let results = [];
for (let [k, v] of formData) results.push([k, v]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if you could just use let results = Array.from(formData) here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants