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: fix the ts version selected by the user #305

Merged
merged 9 commits into from
Feb 19, 2025

Conversation

huangmingfu
Copy link
Contributor

@huangmingfu huangmingfu commented Jan 19, 2025

  • 问题:复现demo
    我在 Vue SFC Playground 发现了一个问题,当我选择了 ts 版本号和 vue 版本号,分享链接出去的时候:
    image
    接着通过分享链接打开后,vue 版本号可以正常的选择用户保存的设置,但是 ts 的始终都是 latest
    image
    image
  • 这个 pr 是修复 ts 版本的选择,我已经在 test/main.ts 测试过,测试代码如下,辛苦审核~
const query = new URLSearchParams(location.search)
const { importMap: builtinImportMap, vueVersion } = useVueImportMap()
vueVersion.value = '3.4.0'
const store = (window.store = useStore(
  {
    builtinImportMap,
    vueVersion,
    showOutput: ref(query.has('so')),
    outputMode: ref((query.get('om') as OutputModes) || 'preview'),
    typescriptVersion: ref('4.9.3')
  },
  location.hash,
))
console.info('store', store)

image

Copy link

vercel bot commented Jan 19, 2025

@huangmingfu is attempting to deploy a commit to the vuejs Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

pkg-pr-new bot commented Feb 18, 2025

Open in Stackblitz

npm i https://pkg.pr.new/@vue/repl@305

commit: 331b60e

src/store.ts Outdated
} else if (filename === '_tsVersion') {
typescriptVersion.value = saved[filename]
}
else {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hope to fix the code format npm run format

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, I've already changed it

src/Repl.vue Outdated
font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Oxygen,
Ubuntu, Cantarell, 'Open Sans', 'Helvetica Neue', sans-serif;
font-family:
-apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Oxygen, Ubuntu,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This may be due to version differences.

Please confirm whether to use pnpm@9 install.
Or undo this irrelevant modification. Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, I've already changed it

src/store.ts Outdated
@@ -270,6 +270,7 @@ export function useStore(
}
}
if (vueVersion.value) files._version = vueVersion.value
if (typescriptVersion.value) files._tsVersion = typescriptVersion.value
Copy link
Collaborator

Choose a reason for hiding this comment

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

🤔 I wonder if typescriptVersion === latest can be ignored ?
By default, all data may have a _tsVersion file.
But the default vueVersion is null.

Copy link
Contributor Author

@huangmingfu huangmingfu Feb 19, 2025

Choose a reason for hiding this comment

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

No, if the ts version is not transmitted, request 404.

const store = (window.store = useStore(
  {
    //  ...
    // typescriptVersion: ref('4.9.3')
  },
  location.hash,
))

image

My latest submission has solved this problem.
image
It is similar to the writing of vue version request.
image

src/store.ts Outdated
@@ -431,7 +434,7 @@ export type StoreState = ToRefs<{

// volar-related
locale: string | undefined
typescriptVersion: string
typescriptVersion: string | null
Copy link
Collaborator

Choose a reason for hiding this comment

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

Type 'string | null' is not assignable to type 'string'.
https://github.com/vuejs/repl/blob/main/src/monaco/env.ts#L140
https://github.com/vuejs/repl/blob/main/src/monaco/env.ts#L77


Perhaps the impact of this change would be minimal.

// L273
if (typescriptVersion.value !== 'latest' || files._tsVersion) {
  files._tsVersion = typescriptVersion.value
}

Copy link
Contributor Author

@huangmingfu huangmingfu Feb 19, 2025

Choose a reason for hiding this comment

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

Forgot to run the typecheck command to check. Thank you for your suggestion. I have already modified it.❤️

@edison1105 edison1105 merged commit 33ca3c0 into vuejs:main Feb 19, 2025
2 of 3 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.

3 participants