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: type error #3863

Merged
merged 1 commit into from
Nov 14, 2023
Merged

fix: type error #3863

merged 1 commit into from
Nov 14, 2023

Conversation

xiaoxiangmoe
Copy link
Contributor

Summary

There are many type errors in the current code that prevent the tsc check of js from being enabled. This PR uses a lot of //@ts-expect-error to bypass type checking errors.

Test Plan

@xiaoxiangmoe xiaoxiangmoe requested a review from a team July 30, 2023 17:07
@CLAassistant
Copy link

CLAassistant commented Jul 30, 2023

CLA assistant check
All committers have signed the CLA.

@xiaoxiangmoe
Copy link
Contributor Author

cc @hardfist

@hyf0
Copy link
Collaborator

hyf0 commented Jul 31, 2023

@web-infra-dev/rspack-dx please take a look.

@stale
Copy link

stale bot commented Sep 29, 2023

This pull request has been automatically marked as stale because it has not had recent activity. If this pull request is still relevant, please leave any comment (for example, "bump").

@stale stale bot added the stale label Sep 29, 2023
@stale stale bot removed the stale label Oct 7, 2023
@hardfist
Copy link
Contributor

bump

@hardfist
Copy link
Contributor

since lots of conflicts here, i will try to fix it in separate PR

packages/rspack/package.json Show resolved Hide resolved
packages/rspack/tsconfig.json Show resolved Hide resolved
packages/rspack-dev-server/src/middleware.ts Outdated Show resolved Hide resolved
@bvanjoi
Copy link
Collaborator

bvanjoi commented Nov 14, 2023

This pr merely introduces some type checkers and does not affect the existing logic. Therefore, I think it can be merged once the ci is happy

@xiaoxiangmoe
Copy link
Contributor Author

@bvanjoi Can you tell me how to fix ci error?

@bvanjoi
Copy link
Collaborator

bvanjoi commented Nov 14, 2023

The failure occurred at rspack-plugin-html. Could you please verify if this issue can be reproduced locally? You can do this by running the following commands: npm run build:cli:release && cd packages/rspack-plugin-html && npm run test.

@xiaoxiangmoe
Copy link
Contributor Author

This issue can be reproduced locally. But I don't know how to fix it. Can you give me some idea about this?

@bvanjoi
Copy link
Collaborator

bvanjoi commented Nov 14, 2023

I suspect that the issue stems from the rspack-plugin-html/dist. Could you revert the changes made to its tsconfig.json, and rebuild, then retest it?

@bvanjoi
Copy link
Collaborator

bvanjoi commented Nov 14, 2023

To be more specific, I believe changing the module to commonjs and moduleResolution to node should resolve this issue.

@hardfist
Copy link
Contributor

it seems hits the nodejs/node#35889 bug again!

auto-merge was automatically disabled November 14, 2023 11:54

Head branch was pushed to by a user without write access

hardfist
hardfist previously approved these changes Nov 14, 2023
auto-merge was automatically disabled November 14, 2023 12:56

Head branch was pushed to by a user without write access

@hardfist hardfist added this pull request to the merge queue Nov 14, 2023
Merged via the queue into web-infra-dev:main with commit dd8f44e Nov 14, 2023
15 checks passed
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.

5 participants