Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

fix(preview): send configured headers #9976

Merged
merged 2 commits into from
Sep 5, 2022
Merged

fix(preview): send configured headers #9976

merged 2 commits into from
Sep 5, 2022
+33 −2

Conversation

bluwy
Copy link
Member

@bluwy bluwy commented Sep 4, 2022

Description

Fix #9864

Update sirv to send headers in preview.

Additional context

The dev equivalent is set as:

vite/packages/vite/src/node/server/middlewares/static.ts

Lines 28 to 42 in f76b1ee

setHeaders(res, pathname) {
// Matches js, jsx, ts, tsx.
// The reason this is done, is that the .ts file extension is reserved
// for the MIME type video/mp2t. In almost all cases, we can expect
// these files to be TypeScript files, and for Vite to serve them with
// this Content-Type.
if (/\.[tj]sx?$/.test(pathname)) {
res.setHeader('Content-Type', 'application/javascript')
}
if (headers) {
for (const name in headers) {
res.setHeader(name, headers[name]!)
}
}
}

I didn't add the ts handling part as those shouldn't happen in preview.

What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@bluwy bluwy added the p3-minor-bug An edge case that only affects very specific usage (priority) label Sep 4, 2022
@bluwy bluwy marked this pull request as draft September 4, 2022 10:57
@bluwy
Copy link
Member Author

bluwy commented Sep 4, 2022

It looks like the test previously failed because the tests aren't using Vite preview, but a custom static server, so I can't test the preview server (though I can confirm it works locally).

@bluwy bluwy marked this pull request as ready for review September 4, 2022 13:52
@patak-dev
Copy link
Member

Not for this PR, but I think we could move the tests to use vite preview, no? They may not be using it only because the preview server is a feature that was developed after the original e2e test infra.

@bluwy
Copy link
Member Author

bluwy commented Sep 5, 2022

Yeah that's definitely a followup we should do. It feels odd that we're not using it.

@bluwy bluwy mentioned this pull request Sep 5, 2022
Merged
@patak-dev patak-dev merged commit 0d20eae into main Sep 5, 2022
@patak-dev patak-dev deleted the fix-preview-headers branch September 5, 2022 14:55
@f0o
Copy link

f0o commented Jan 12, 2024

Should this still be working?

Because for the life of me I cannot figure out how to add custom response headers onto vite's preview.

Config:

export default defineConfig({
    plugins: [sveltekit()],
    preview: {
        headers: {
            "X-Foo": "bar",
        }
    }
});

Test:

$ curl -i http://localhost:4173/
HTTP/1.1 200 OK
Access-Control-Allow-Origin: *
content-type: text/html
etag: "1705064869377"
Date: Fri, 12 Jan 2024 13:30:51 GMT
Connection: keep-alive
Keep-Alive: timeout=5
Transfer-Encoding: chunked

<!DOCTYPE html>
...

@bluwy
Copy link
Member Author

bluwy commented Jan 15, 2024

@f0o I can't replicate that locally. Could it be a SvelteKit specific issue?

@f0o
Copy link

f0o commented Jan 16, 2024

@bluwy I guess, neither preview nor server settings seem to do anything for me. Wonder how the SvelteKit plugin manages to intercept/override those

# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@patak-dev patak-dev patak-dev approved these changes

Assignees
No one assigned
Labels
p3-minor-bug An edge case that only affects very specific usage (priority)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

vite preview … server does not send configured headers
3 participants
@bluwy @patak-dev @f0o