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

Add support for byte-range requests #62

Merged
merged 1 commit into from
Apr 19, 2024
Merged

Add support for byte-range requests #62

merged 1 commit into from
Apr 19, 2024

Conversation

joemaller
Copy link
Contributor

Eleventy Dev Server does not currently support byte-range requests, which causes locally served video files to fail on Safari and all iOS browsers.

This PR adds support for byte-range requests, so local video files load correctly from partial byte-range requests. This fixes #51.

Rather than trying to implement a new byte-range handler, I suggest the using the send library from PillarJS. This project already relies on their finalhandler library. These two libraries each log about 25M downloads per week.

I recognize, respect and appreciate the desire to keep 11ty's installations small. Send has 13 dependencies, 7 of which are already utilized and installed by eleventy-dev-server.

With send, byte-range requests can be implemented by simply adding a range-header check to eleventyProjectMiddleware:

if (req.headers.range) {
  return send(req, match.filepath).pipe(res);
}

Tests

The fetchHeadersForRequest test helper function was slightly modified. It now accepts an extras argument, which can be used to add additional http.request options. This allows the new tests to include a range header. The function's "Invalid status code" error-check was also modified to recognize success codes 200 and 206.

Changes to the tests were checked and verified before adding range-requests to server.js, all tests passed.

Two tests specific to this change were added. The first validates byte-range headers are returned correctly, the second assures byte-range headers do not appear on normal requests.

Also note that options.portReassignmentRetryCount was increased to prevent the test suite from running out of ports.

@noelforte
Copy link

@joemaller THANK YOU. I set an override in my package.json to use your version for now and it works beautifully. Just ran across this while doing some local development on my site and could not figure out why I couldn't see my <video>s on my phone...dang it Safari. 😂 Glad to know I wasn't the only one!

Adding my support for this pull (which is about 9 months old at this point). @zachleat (or maintainers), is there any way we could get this merged to mitigate this pain point?

@zachleat zachleat merged commit fc4c3cb into 11ty:main Apr 19, 2024
@zachleat
Copy link
Member

Shipping with 2.0.0 thank you!

@zachleat zachleat added this to the Eleventy Dev Server v2.0.0 milestone Apr 19, 2024
@zachleat zachleat added the bug Something isn't working label Apr 19, 2024
@jmuspratt
Copy link

jmuspratt commented May 9, 2024

@joemaller THANK YOU. I set an override in my package.json to use your version for now and it works beautifully. Just ran across this while doing some local development on my site and could not figure out why I couldn't see my <video>s on my phone...dang it Safari. 😂 Glad to know I wasn't the only one!

Adding my support for this pull (which is about 9 months old at this point). @zachleat (or maintainers), is there any way we could get this merged to mitigate this pain point?

@noelforte Do you mind posting the relevant part of your package.json? My project is on eleventy 2.0.1 and I can't get seem to solve this either with an override or a dev dependency (@11ty/eleventy-dev-server": "^2.0.0",) to work. (The dev dependency seems to successfully install the 2.0.0 in my node_modules but doesn't get the videos playing in Safari). Thanks!

@noelforte
Copy link

@jmuspratt Remember to refresh your lockfile when editing overrides! This was oversight I ran into myself, so I'm betting that's why it's not working on your end.

I put together a reduced case over at https://github.com/noelforte/11ty-range-rtc to confirm that using eleventy-dev-server@^2.0.0 is possible with older versions of Eleventy. Hope this helps!

@jmuspratt
Copy link

jmuspratt commented May 9, 2024

@noelforte You are a gentleman and a scholar. That worked! And the demo helped narrow the actual issue: yarn does not support overrides 🤦 . (The equivalent in yarn is resolutions). Switching to npm and sticking with overrides worked for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Byte-range requests are not in HTTP headers, so Safari can't play video files served by Eleventy Server
4 participants