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
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
93406ed
fix(server-renderer): getSSRProps can get exposed property
baiwusanyu-c Jan 11, 2023
a8aaa17
fix(server-renderer): added unit test
baiwusanyu-c Jan 11, 2023
43aa9e1
fix(server-renderer): update code
baiwusanyu-c Jan 11, 2023
fb80c84
Merge branch 'vuejs:main' into bwsy/fix/getSSRProps
baiwusanyu-c Jan 14, 2023
5f43eed
Merge branch 'vuejs:main' into bwsy/fix/getSSRProps
baiwusanyu-c Feb 4, 2023
399c1ce
Merge branch 'vuejs:main' into bwsy/fix/getSSRProps
baiwusanyu-c Feb 6, 2023
b174489
Merge branch 'main' into bwsy/fix/getSSRProps
baiwusanyu-c Feb 14, 2023
1bfa639
Merge branch 'main' into bwsy/fix/getSSRProps
baiwusanyu-c Feb 21, 2023
a581f3c
Merge branch 'main' into bwsy/fix/getSSRProps
baiwusanyu-c Feb 22, 2023
8976c26
Merge branch 'main' into bwsy/fix/getSSRProps
baiwusanyu-c Mar 17, 2023
1b85487
Merge branch 'main' into bwsy/fix/getSSRProps
baiwusanyu-c Mar 20, 2023
2365226
Merge branch 'main' into bwsy/fix/getSSRProps
baiwusanyu-c Mar 23, 2023
68bd92c
Merge branch 'main' into bwsy/fix/getSSRProps
baiwusanyu-c Mar 27, 2023
33ad78c
Merge remote-tracking branch 'origin/main' into bwsy/fix/getSSRProps
baiwusanyu-c Apr 5, 2023
7d0bbff
fix(server-render): updated unit test
baiwusanyu-c Apr 5, 2023
be00941
Merge remote-tracking branch 'origin/main' into bwsy/fix/getSSRProps
baiwusanyu-c Apr 6, 2023
53aaee6
Merge branch 'main' into bwsy/fix/getSSRProps
baiwusanyu-c Apr 6, 2023
671aaad
Merge branch 'main' into bwsy/fix/getSSRProps
baiwusanyu-c Apr 10, 2023
ee51e5e
Merge branch 'main' into bwsy/fix/getSSRProps
baiwusanyu-c Apr 12, 2023
93eecc4
Merge branch 'main' into bwsy/fix/getSSRProps
baiwusanyu-c Apr 14, 2023
9029a9a
Merge branch 'main' into bwsy/fix/getSSRProps
baiwusanyu-c Apr 17, 2023
a57a19b
Merge branch 'main' into bwsy/fix/getSSRProps
baiwusanyu-c Apr 19, 2023
9209661
Merge branch 'main' into bwsy/fix/getSSRProps
baiwusanyu-c Apr 20, 2023
c4522a5
Merge branch 'main' into bwsy/fix/getSSRProps
baiwusanyu-c May 4, 2023
cc7b894
Merge branch 'main' into bwsy/fix/getSSRProps
baiwusanyu-c May 16, 2023
b263a8c
Merge branch 'main' into bwsy/fix/getSSRProps
baiwusanyu-c May 17, 2023
27d26cf
Merge branch 'main' into bwsy/fix/getSSRProps
baiwusanyu-c Jun 6, 2023
0223c97
Merge remote-tracking branch 'origin/main' into bwsy/fix/getSSRProps
baiwusanyu-c Jan 3, 2024
f103ba5
[autofix.ci] apply automated fixes
autofix-ci[bot] Jan 3, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/runtime-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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


// For raw render function users
export { h } from './h'
Expand Down
47 changes: 46 additions & 1 deletion packages/server-renderer/__tests__/ssrDirectives.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ import {
vModelRadio,
vModelCheckbox,
vModelDynamic,
resolveDirective
resolveDirective,
mergeProps,
ref
} from 'vue'
import { ssrGetDirectiveProps, ssrRenderAttrs } from '../src'

Expand Down Expand Up @@ -522,4 +524,47 @@ describe('ssr: directives', () => {
)
).toBe(`<div id="foo-arg-true"></div>`)
})

// #7499
test('custom directive w/ getSSRProps (expose)', async () => {
let exposeVars: null | string | undefined = null
const useTestDirective = () => ({
vTest: {
getSSRProps({ instance }: any) {
if (instance) {
exposeVars = instance.x
}
return { id: exposeVars }
}
}
})
const { vTest } = useTestDirective()

const renderString = await renderToString(
createApp({
setup(props, { expose }) {
const x = ref('foo')
expose({ x })
const __returned__ = { useTestDirective, vTest, ref, x }
Object.defineProperty(__returned__, '__isScriptSetup', {
enumerable: false,
value: true
})
return __returned__
},
ssrRender(_ctx, _push, _parent, _attrs, $props, $setup) {
_push(
`<div${ssrRenderAttrs(
mergeProps(
_attrs!,
ssrGetDirectiveProps(_ctx, $setup['vTest'] as any)
)
)}></div>`
)
}
})
)
expect(renderString).toBe(`<div id="foo"></div>`)
expect(exposeVars).toBe('foo')
})
})
4 changes: 2 additions & 2 deletions packages/server-renderer/src/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import {
import { ssrRenderAttrs } from './helpers/ssrRenderAttrs'
import { ssrCompile } from './helpers/ssrCompile'
import { ssrRenderTeleport } from './helpers/ssrRenderTeleport'

import { getExposeProxy } from '@vue/runtime-dom'
const {
createComponentInstance,
setCurrentRenderingInstance,
Expand Down Expand Up @@ -183,7 +183,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)

push,
instance,
attrs,
Expand Down