Skip to content

chore: revert #10196 until Vite 4#10574

Merged
patak-cat merged 1 commit intomainfrom
fix/revert-10196
Oct 21, 2022
Merged

chore: revert #10196 until Vite 4#10574
patak-cat merged 1 commit intomainfrom
fix/revert-10196

Conversation

@patak-cat
Copy link
Copy Markdown
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
Copy Markdown
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-cat
Copy link
Copy Markdown
Member Author

@PengBoUESTC
Copy link
Copy Markdown
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-cat
Copy link
Copy Markdown
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
Copy Markdown
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-cat patak-cat merged commit 07c0336 into main Oct 21, 2022
@patak-cat patak-cat 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