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

Fix Cloudflare Workers adapter issues #946

Merged
merged 3 commits into from
Apr 13, 2021
Merged

Conversation

leesus
Copy link
Contributor

@leesus leesus commented Apr 10, 2021

Prevents the Cloudflare Workers adapter re-stringifying a JSON request body after parsing with await request.json().

Currently, defensive code needs to be written when running locally vs running in a worker, e.g.

export function post(request) {
  let data;

  try {
    // Workers
    ({ data } = JSON.parse(request.body));
  } catch (e) {
    // Locally
    ({ data } = request.body);
  }
  
  ...
}

instead of

export function post(request) {
  const { data } = request.body;

  ...
}

Converts Headers object to POJO prior to passing to render method, so that getContext hooks etc can access properties via dot notation, rather than using get/has Headers methods.

Prevent the Cloudflare Workers adapter re-stringifying a JSON request
body after parsing with `await request.json()`.
@leesus leesus changed the title Prevent re-stringifying JSON request body in Cloudflare Workers adapter Fix Cloudflare Workers adapter issues Apr 10, 2021
@benmccann
Copy link
Member

@leesus can you add a changeset? (i.e. run pnpx changeset)

@leesus
Copy link
Contributor Author

leesus commented Apr 13, 2021

@benmccann done 🙂

Copy link
Contributor

@halfnelson halfnelson left a comment

Choose a reason for hiding this comment

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

Looks good, as long as the headers object still matches the type Def's from kit

@leesus
Copy link
Contributor Author

leesus commented Apr 13, 2021

@halfnelson this converts the Headers instance from Workers into a plain JS object, which looks to be correct as per types.internal.d.ts (Record<string, string>).

@benmccann benmccann merged commit 4325b39 into sveltejs:master Apr 13, 2021
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

3 participants