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(static): serve .js, .jsx, .ts, .tsx as application/javascript #2769

Merged
merged 1 commit into from
Mar 31, 2021

Conversation

eirslett
Copy link
Contributor

This fixes #2642. (Well, I believe it fixes it)
Thanks a lot to @lukeed, maintainer of Sirv, for the suggestion!
lukeed/sirv#103 (comment)

To be honest, I had a hard time finding a good way to test this fix - npm link didn't work for me, and I couldn't find any integration tests or documentation on how to develop locally using a forked version of Vite. Could somebody please point me in the right direction?

import sirv from 'sirv'
import { Connect } from 'types/connect'
import { ResolvedConfig } from '../..'
import { FS_PREFIX } from '../../constants'
import { cleanUrl, isImportRequest } from '../../utils'

const sirvOptions = { dev: true, etag: true, extensions: [] }
const sirvOptions = {
Copy link

Choose a reason for hiding this comment

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

You can import type { Options } from 'sirv'; and then

Suggested change
const sirvOptions = {
const sirvOptions: Options = {

This means you don't have to import ServerResponse or type the setHeaders method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion! I updated this PR now.

This fixes vitejs#2642.
Thanks a lot to @lukeed, maintainer of Sirv, for the suggestion!
lukeed/sirv#103 (comment)
@patak-dev patak-dev added the p2-nice-to-have Not breaking anything but nice to have (priority) label Mar 29, 2021
@patak-dev
Copy link
Member

Hey @eirslett, thanks for the PR!

To test this, you can use yarn link, like explained here https://vitejs.dev/guide/#using-unreleased-commits
@ElMassimo also wrote this guide for Vite https://maximomussini.com/posts/debugging-javascript-libraries

cd vite
cd packages/vite
yarn
yarn link
yarn dev

Then you can link the project you want to debug with this local copy using yarn link vite in that project folder. In VS Code, you can start in a Javascript debugging console, and run yarn dev in that project folder, and you can set up breakpoints in Vite's code if needed.

// 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)) {
Copy link
Member

@Shinigami92 Shinigami92 Mar 30, 2021

Choose a reason for hiding this comment

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

thought/question: Could cjs and mjs also be valid considerations? 🤔


ext kind mime
mjs JavaScript module text/javascript
cjs unknown unknown

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Possibly, I'm not sure what the maintainers would want it to be like... maybe it's better to do in a separate PR, at least now it's easy to find where you should add it in the code!

@eirslett
Copy link
Contributor Author

@patak-js thanks a lot for the help! I was able to test it locally now, and it works like intended!

Copy link
Member

@patak-dev patak-dev left a comment

Choose a reason for hiding this comment

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

I also tested it and it works great. Thanks a lot for the PR 💯

We may need to review as suggested if we should include other extensions in future PRs

We have an active #contributing channel in Vite Land, in case you would like to join. The debugging links were part of a discussion we had the day before there 😃

@patak-dev patak-dev merged commit b08e973 into vitejs:main Mar 31, 2021
@eirslett eirslett deleted the typescript-mime-type branch March 31, 2021 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p2-nice-to-have Not breaking anything but nice to have (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The dev server uses mime type "video/mp2t"
4 participants