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

chore: revert #10196 until Vite 4 #10574

Merged
merged 1 commit into from
Oct 21, 2022
Merged

chore: revert #10196 until Vite 4 #10574

merged 1 commit into from
Oct 21, 2022

Conversation

patak-dev
Copy link
Member

This reverts commit f328f61.

Description

Marko is failing after #10196

@PengBoUESTC we can do this change in Vite 4 in a month, so we don't introduce a breaking change (even if it is just types) in a minor.

Error: src/__tests__/main.test.ts(156,36): error TS2345: Argument of type 'Server | Http2SecureServer' is not assignable to parameter of type 'Server'.
  Type 'Http2SecureServer' is missing the following properties from type 'Server': maxHeadersCount, maxRequestsPerSocket, timeout, headersTimeout, and 2 more.

The error is strange, why Http2SecureServer is missing properties from Server?


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

@PengBoUESTC
Copy link
Contributor

PengBoUESTC commented Oct 21, 2022

in https://github.com/marko-js/vite/blob/main/src/__tests__/main.test.ts, this function shown below accepts a param server with a type http.Server.

async function testPage(dir: string, steps: Step[], server: http.Server) {

I don't think that will be a problem..

@patak-dev
Copy link
Member Author

@PengBoUESTC Marko is failing here https://github.com/vitejs/vite-ecosystem-ci/actions/runs/3295058377/jobs/5433215758, Http2SecureServer doesn't extends Server

@PengBoUESTC
Copy link
Contributor

@PengBoUESTC Marko is failing here https://github.com/vitejs/vite-ecosystem-ci/actions/runs/3295058377/jobs/5433215758, Http2SecureServer doesn't extends Server

import type { Http2SecureServer } from 'node:http2'
http.Server
I think it means that http2.Http2SecureServer is not assignable to http.Server, that's correct,

in Marko-js/vite testPage function, the server parameter is declared as http.Server
async function testPage(dir: string, steps: Step[], server: http.Server) {

I think Marko-js/vite should update the server type when they want to update the vite version

@patak-dev
Copy link
Member Author

I think Marko-js/vite should update the server type when they want to update the vite version

Yes, this is why we think it is better to move this PR to Vite 4. There weren't enough requests to justify having marko deal with this breaking change in their types in a minor version.

@PengBoUESTC
Copy link
Contributor

I think Marko-js/vite should update the server type when they want to update the vite version

Yes, this is why we think it is better to move this PR to Vite 4. There weren't enough requests to justify having marko deal with this breaking change in their types in a minor version.

Ok that maybe a better choice !

@patak-dev patak-dev merged commit 07c0336 into main Oct 21, 2022
@patak-dev patak-dev deleted the fix/revert-10196 branch October 21, 2022 09:42
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