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(server-renderer): getSSRProps can get exposed property #7502

Closed
wants to merge 29 commits into from

Conversation

baiwusanyu-c
Copy link
Member

close: #7499

@baiwusanyu-c baiwusanyu-c marked this pull request as draft January 11, 2023 05:49
@baiwusanyu-c baiwusanyu-c marked this pull request as ready for review January 11, 2023 05:50
@@ -79,7 +79,7 @@ export {

// For getting a hold of the internal instance in setup() - useful for advanced
// plugins
export { getCurrentInstance } from './component'
export { getCurrentInstance, getExposeProxy } from './component'
Copy link
Member

@LinusBorg LinusBorg Jan 11, 2023

Choose a reason for hiding this comment

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

I'm not sure that we should expose this from runtime-core, as this way it will be re-exposed from the main vue package. But really it's an internal API.

I think we should move to the shared package like other stuff that's internally shared between different renderers.

Copy link
Member Author

Choose a reason for hiding this comment

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

What you said makes sense, but moving getExposeProxy to shaerd seems to be more costly, because the publicPropertiesMap it uses involves a lot of internal APIs.
Import getExposeProxy directly through the path, how do you think this is possible?

import { getExposeProxy } from '../../runtime-core/src/component'

Because I see that there are similar writing in customFormatter.ts and packages/vue-compat/src/index.ts

@ThornWalli
Copy link

What is the current status here?

@yangwao
Copy link

yangwao commented Feb 17, 2023

Hey, how is this standing? I've heard nuxt-speedkit would need it and then kodadot/nft-gallery is happy to use it 😁

@StephanGerbeth
Copy link

@baiwusanyu-c @LinusBorg when will you merge the fix into the main branch?

@vercel
Copy link

vercel bot commented Apr 6, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Updated (UTC)
sfc-playground ⬜️ Ignored (Inspect) Apr 6, 2023 0:44am

@ThornWalli
Copy link

@baiwusanyu-c Thanks to the PR 🙂

Now this PR is almost half a year old, not really feel like waiting until this reaches its first year of life 😉

Seems to be only minimal changes and slowly the excitement rises.

Could you also alternatively use a workaround here for now without this PR?
Is that possible?

baiwusanyu-c and others added 2 commits January 3, 2024 11:15
# Conflicts:
#	packages/server-renderer/__tests__/ssrDirectives.spec.ts
Copy link

github-actions bot commented Jan 3, 2024

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 89.5 kB 34 kB 30.7 kB
vue.global.prod.js 146 kB 53.3 kB 47.6 kB

Usages

Name Size Gzip Brotli
createApp 49.8 kB 19.5 kB 17.8 kB
createSSRApp 53.1 kB 20.8 kB 19 kB
defineCustomElement 52.1 kB 20.3 kB 18.5 kB
overall 63.3 kB 24.4 kB 22.3 kB

@@ -188,7 +188,7 @@ function renderComponentSubTree(
const prev = setCurrentRenderingInstance(instance)
try {
ssrRender(
instance.proxy,
getExposeProxy(instance) || instance.proxy,
Copy link
Member

Choose a reason for hiding this comment

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

This isn't really the right fix as for the component's own render function, it should be using the default (internal facing) proxy.

See the correct fix in df686ab (reusing tests from this PR)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Rejected
Development

Successfully merging this pull request may close these issues.

Directive method getSSRProps has no access to exposed property on the instance
7 participants