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

Spec edits for multipart @defer @stream around content-length #152

Merged
merged 4 commits into from
Nov 18, 2020
Merged

Spec edits for multipart @defer @stream around content-length #152

merged 4 commits into from
Nov 18, 2020

Conversation

maraisr
Copy link
Contributor

@maraisr maraisr commented Nov 18, 2020

This PR aims to amend a few things that were discovered upon implementation:

  • Content-Length is not mentioned as a requirement from the multipart spec. The change will now reflect that the boundary is not the "start" of the part, but rather the end of the part. The first boundary being the end of the preamble. This will give clients a much more predictable "I have enough body" to then begin parsing. Rather than wait for the next boundary to flush, which as per sample implementations wouldn't flush till the next chunk was written, potentially delaying the processing of that part. This now means a client can consume chunks till a boundary, then parse.
  • the empty line after a body is also not accurate per multipart spec.

This will mean we're seeing a much tighter smaller payload, and easier to consume for clients.

cc @robrichard

@maraisr maraisr changed the title feat: Sprinkle spec edits Spec edits for multipart @defer @stream around conent-length Nov 18, 2020
@maraisr maraisr changed the title Spec edits for multipart @defer @stream around conent-length Spec edits for multipart @defer @stream around content-length Nov 18, 2020
@mmatsa mmatsa merged commit 290b0e2 into graphql:master Nov 18, 2020
@maraisr maraisr deleted the patch-1 branch November 18, 2020 04:12
acao pushed a commit to graphql/express-graphql that referenced this pull request Dec 2, 2020
This removes the need for `Content-Length`, and handles the boundaries such that the next boundary is flushed as soon as possible.

References:
- graphql/graphql-over-http#152
- fmg relay-tools/fetch-multipart-graphql#22
- [meros](https://github.com/maraisr/meros)
robrichard pushed a commit to graphql/express-graphql that referenced this pull request Dec 3, 2020
This removes the need for `Content-Length`, and handles the boundaries such that the next boundary is flushed as soon as possible.

References:
- graphql/graphql-over-http#152
- fmg relay-tools/fetch-multipart-graphql#22
- [meros](https://github.com/maraisr/meros)
robrichard pushed a commit to graphql/express-graphql that referenced this pull request Feb 9, 2021
This removes the need for `Content-Length`, and handles the boundaries such that the next boundary is flushed as soon as possible.

References:
- graphql/graphql-over-http#152
- fmg relay-tools/fetch-multipart-graphql#22
- [meros](https://github.com/maraisr/meros)
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.

3 participants