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: compatibility with vue-tsc 2.x #327

Merged
merged 34 commits into from
Jun 29, 2024
Merged

Conversation

so1ve
Copy link
Contributor

@so1ve so1ve commented Jun 21, 2024

fixes #306, close #321

This is a naive fix - it may cause problems described in #306 (comment), but it will avoid crashes.

I'm still working on a complete solution.

Fixed, but requires installing @vue/language-core as a dependency, which may cause version mismatch.

Ran e2e tests locally and passed, the ci is failing due to d ts building issues and low nodejs version

Copy link

changeset-bot bot commented Jun 21, 2024

🦋 Changeset detected

Latest commit: ef0479f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
vite-plugin-checker Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

netlify bot commented Jun 21, 2024

Deploy Preview for vite-plugin-checker ready!

Name Link
🔨 Latest commit ef0479f
🔍 Latest deploy log https://app.netlify.com/sites/vite-plugin-checker/deploys/667f8e4bf7c0fc0008dd60dd
😎 Deploy Preview https://deploy-preview-327--vite-plugin-checker.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@fi3ework
Copy link
Owner

fi3ework commented Jun 22, 2024

I checked the PR code and found vscode-uri blocks test:unit. It has been bumped to 3.0.8 in lockfile and the type exports has been fixed. We could change https://github.com/fi3ework/vite-plugin-checker/blob/0a2a2e04a79bfb90c38f1458467c749978c7cdbb/packages/vite-plugin-checker/src/checkers/vls/diagnostics.ts#L27 and https://github.com/so1ve/vite-plugin-checker/blob/main/packages/vite-plugin-checker/src/checkers/vls/diagnostics.ts#L27-L28 to normal write up.

@so1ve
Copy link
Contributor Author

so1ve commented Jun 22, 2024

Invalid back references breaks dts build, any thoughts?

@fi3ework
Copy link
Owner

Invalid back references breaks dts build, any thoughts?

Just follow the new version of typescript's instruction and change to const myRegexp = /([^\s'"]([^\s'"]*(['"])([^\x03]*?)\3)+[^\s'"]*)|[^\s'"]+|(['"])([^\x05]*?)\5/gi would be fine.

@so1ve
Copy link
Contributor Author

so1ve commented Jun 22, 2024

I suppose \x03 is hex while \3 is a back reference?

@fi3ework
Copy link
Owner

I also found that multiple-hmr and multiple-reload test case has some existing flaw which will break in this PR as it's using the project's thought ESLint config. I think it could be rewritten based on eslint-default and add one line in the main.ts to trigger TS error. And update vite.config.ts alongside is enough. And the snapshot could be updated totally.

import { text } from './text'

+ let count: string = 0
var hello = 'Hello'

const rootDom = document.querySelector('#root')!
rootDom.innerHTML = hello + text

export {}
import { defineConfig } from 'vite'
import checker from 'vite-plugin-checker'

export default defineConfig({
  plugins: [
    checker({
+      typescript: true,
      eslint: {
        lintCommand: 'eslint ./src --ext .ts',
      },
    }),
  ],
})

@fi3ework
Copy link
Owner

fi3ework commented Jun 22, 2024

I suppose \x03 is hex while \3 is a back reference?

Guess we could ts-expect-error as per microsoft/TypeScript#58416.

@fi3ework
Copy link
Owner

@so1ve Could you help to check the Windows CI issue, it's relatively slow for me to test it on CI and I can't reach my Windows machine until next week. 🥺

CoreNion added a commit to CoreNion/cp-dashboard that referenced this pull request Jun 27, 2024
関連のIssue:
fi3ework/vite-plugin-checker#306 (comment)
vuejs/language-tools#4484

一時的な回避案で、正式な修正は`vite-plugin-checker`のvue-tsc v2.x対応待ち
fi3ework/vite-plugin-checker#327
@so1ve
Copy link
Contributor Author

so1ve commented Jun 27, 2024

Hi @fi3ework, I don't have enough time to work on windows tests recently, but I can work on this issue in two days. How about we merge this PR first, and then I will submit another PR to fix it?

@fi3ework
Copy link
Owner

fi3ework commented Jun 28, 2024

Hi @fi3ework, I don't have enough time to work on windows tests recently, but I can work on this issue in two days. How about we merge this PR first, and then I will submit another PR to fix it?

Sure, I'll merge this PR tonight (I'll try to fix the Windows CI one more time before that 🥲).

@fi3ework
Copy link
Owner

GitHub is down https://www.githubstatus.com/incidents/9vwllhs2w1kj, I can't merge the PR right now. It's late tonight, I'll try to merge this PR tomorrow morning.

Google Chrome 2024-06-29 02 07 03

@fi3ework fi3ework merged commit 0747729 into fi3ework:main Jun 29, 2024
7 of 8 checks passed
@fi3ework
Copy link
Owner

fi3ework commented Jun 29, 2024

@so1ve Thanks for the hard work, I really appreciate that. Looking forward to releasing this after fixing the left comment.

  • CI failed on Windows
  • vue-tsc^2 not works with ts^4 (expected?)

@armano2
Copy link
Contributor

armano2 commented Jun 29, 2024

windows test seem to be failing due to jest-serializer-path normalization, path drive path is replaced with val => val.replace(/[a-zA-Z]:\\/g, "\\") and this seem to not be present in https://github.com/fi3ework/vite-plugin-checker/blob/56761d092f4c80f321da96bb1bab755f10edc45c/playground/serializers.ts

i created a PR, that hack "fixes" this issue #313

@so1ve
Copy link
Contributor Author

so1ve commented Jun 29, 2024

Hi @fi3ework, yes, we don't support ts 4. We rely on ts 5 specific apis.

@fi3ework
Copy link
Owner

fi3ework commented Jun 29, 2024

@so1ve Hmm, I'm wondering why peer dep is set * of vue-tsc https://www.npmjs.com/package/vue-tsc?activeTab=code

@so1ve
Copy link
Contributor Author

so1ve commented Jun 29, 2024

Yeah, it would be better to point it to ^5. Will do, thanks!

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.

Stopped working with vue-tsc 2.0.1
5 participants