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(types): ensure createBlock() helper accepts Teleport and Supsense types (fix: #2855) #5458

Merged
merged 2 commits into from Oct 20, 2022

Conversation

LinusBorg
Copy link
Member

ensure createBlock() helper accepts Teleport and Supsense types, not their internal counterparts (TeleportImpl and SupenseImpl).

createBlock() only accepts VNodeTypes as first argument, and this type only included the internal types of Teleport and Suspense - TeleportImpl and SuspenseImpl.

This PR ads the public types Teleport and Suspense and thus ensures that the following, compiler-generated code now passes in vue-tsc and Volar:

return (_ctx, _cache) => {
  return (_openBlock(), _createBlock(_Teleport, { to: "body" }, [
    _createTextVNode(" Foo " + _toDisplayString(_ctx.title), 1 /* TEXT */)
  ]))
}

fix: #2855

… types

not their internal counterparts (TeleportImpl and SupenseImpl)

fix: #2855
@netlify
Copy link

netlify bot commented Feb 19, 2022

✔️ Deploy Preview for vue-sfc-playground ready!

🔨 Explore the source changes: f064263

🔍 Inspect the deploy log: https://app.netlify.com/sites/vue-sfc-playground/deploys/621216212e54bb0007637b87

😎 Browse the preview: https://deploy-preview-5458--vue-sfc-playground.netlify.app/

@netlify
Copy link

netlify bot commented Feb 19, 2022

✔️ Deploy Preview for vuejs-coverage ready!

🔨 Explore the source changes: f064263

🔍 Inspect the deploy log: https://app.netlify.com/sites/vuejs-coverage/deploys/62121620701b550007932086

😎 Browse the preview: https://deploy-preview-5458--vuejs-coverage.netlify.app

@netlify
Copy link

netlify bot commented Feb 19, 2022

✔️ Deploy Preview for vue-next-template-explorer ready!

🔨 Explore the source changes: f064263

🔍 Inspect the deploy log: https://app.netlify.com/sites/vue-next-template-explorer/deploys/621216212e54bb0007637b85

😎 Browse the preview: https://deploy-preview-5458--vue-next-template-explorer.netlify.app

@LinusBorg LinusBorg requested a review from pikax February 19, 2022 09:58
@LinusBorg
Copy link
Member Author

@pikax could you have a look at this? Also, any idea where to test this? I didn't find any other test-dts tests for these compiler-helpers like createBlock() so not sure where to put them/how to do them.

@LinusBorg LinusBorg added 🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. scope: types ready to merge The PR is ready to be merged. labels Feb 19, 2022
@pikax
Copy link
Member

pikax commented Feb 19, 2022

@pikax could you have a look at this? Also, any idea where to test this? I didn't find any other test-dts tests for these compiler-helpers like createBlock() so not sure where to put them/how to do them.

It looks good to me, we usually don't have test for internal stuff, since the createBlock would be generated by the compiler, we can add a new test for this, probably for compiler generated stuff.

@LinusBorg
Copy link
Member Author

we usually don't have test for internal stuff, since the createBlock would be generated by the compiler

That wasn't relevant when we didn't really offer typechecking for the template/generated render function, but now with Volar and vue-tsc we effectively do, and the issue addressed by this PR is a result of that.

Seems we generally need a few tests in that area.

@LinusBorg LinusBorg added this to Approved in Next Patch Mar 10, 2022
@yyx990803 yyx990803 removed this from Approved by a team member in Next Patch May 11, 2022
@d0peCode
Copy link

d0peCode commented Sep 6, 2022

This changes would make possible building teleport and suspense with vite. Currently we have workaround (binding teleport component) which makes it ugly and difficult to unit test (stub not replacing binded teleport component in time).

@LinusBorg LinusBorg merged commit e5fc7dc into main Oct 20, 2022
@LinusBorg LinusBorg deleted the linusborg/fix-2855-Teleport-Type-in-template branch October 20, 2022 19:45
chrislone pushed a commit to chrislone/core that referenced this pull request Feb 4, 2023
… types (fix: vuejs#2855) (vuejs#5458)

Co-authored-by: Carlos Rodrigues <carlos@hypermob.co.uk>
zhangzhonghe pushed a commit to zhangzhonghe/core that referenced this pull request Apr 12, 2023
… types (fix: vuejs#2855) (vuejs#5458)

Co-authored-by: Carlos Rodrigues <carlos@hypermob.co.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. ready to merge The PR is ready to be merged. scope: types
Projects
None yet
Development

Successfully merging this pull request may close these issues.

An type error occurs when building Teleport using the <script setup lang="ts">
4 participants