-
-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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
feat(runtime-core): useId() #11404
+294
−0
Merged
feat(runtime-core): useId() #11404
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,188 @@ | ||
/** | ||
* @vitest-environment jsdom | ||
*/ | ||
import { | ||
type App, | ||
Suspense, | ||
createApp, | ||
defineAsyncComponent, | ||
defineComponent, | ||
h, | ||
useId, | ||
} from 'vue' | ||
import { renderToString } from '@vue/server-renderer' | ||
|
||
type TestCaseFactory = () => [App, Promise<any>[]] | ||
|
||
async function runOnClient(factory: TestCaseFactory) { | ||
const [app, deps] = factory() | ||
const root = document.createElement('div') | ||
app.mount(root) | ||
await Promise.all(deps) | ||
await promiseWithDelay(null, 0) | ||
return root.innerHTML | ||
} | ||
|
||
async function runOnServer(factory: TestCaseFactory) { | ||
const [app, _] = factory() | ||
return (await renderToString(app)) | ||
.replace(/<!--[\[\]]-->/g, '') // remove fragment wrappers | ||
.trim() | ||
} | ||
|
||
async function getOutput(factory: TestCaseFactory) { | ||
const clientResult = await runOnClient(factory) | ||
const serverResult = await runOnServer(factory) | ||
expect(serverResult).toBe(clientResult) | ||
return clientResult | ||
} | ||
|
||
function promiseWithDelay(res: any, delay: number) { | ||
return new Promise<any>(r => { | ||
setTimeout(() => r(res), delay) | ||
}) | ||
} | ||
|
||
const BasicComponentWithUseId = defineComponent({ | ||
setup() { | ||
const id1 = useId() | ||
const id2 = useId() | ||
return () => [id1, ' ', id2] | ||
}, | ||
}) | ||
|
||
describe('useId', () => { | ||
test('basic', async () => { | ||
expect( | ||
await getOutput(() => { | ||
const app = createApp(BasicComponentWithUseId) | ||
return [app, []] | ||
}), | ||
).toBe('v0:0 v0:1') | ||
}) | ||
|
||
test('async component', async () => { | ||
const factory = ( | ||
delay1: number, | ||
delay2: number, | ||
): ReturnType<TestCaseFactory> => { | ||
const p1 = promiseWithDelay(BasicComponentWithUseId, delay1) | ||
const p2 = promiseWithDelay(BasicComponentWithUseId, delay2) | ||
const AsyncOne = defineAsyncComponent(() => p1) | ||
const AsyncTwo = defineAsyncComponent(() => p2) | ||
const app = createApp({ | ||
setup() { | ||
const id1 = useId() | ||
const id2 = useId() | ||
return () => [id1, ' ', id2, ' ', h(AsyncOne), ' ', h(AsyncTwo)] | ||
}, | ||
}) | ||
return [app, [p1, p2]] | ||
} | ||
|
||
const expected = | ||
'v0:0 v0:1 ' + // root | ||
'v1:0 v1:1 ' + // inside first async subtree | ||
'v2:0 v2:1' // inside second async subtree | ||
// assert different async resolution order does not affect id stable-ness | ||
expect(await getOutput(() => factory(10, 20))).toBe(expected) | ||
expect(await getOutput(() => factory(20, 10))).toBe(expected) | ||
}) | ||
|
||
test('serverPrefetch', async () => { | ||
const factory = ( | ||
delay1: number, | ||
delay2: number, | ||
): ReturnType<TestCaseFactory> => { | ||
const p1 = promiseWithDelay(null, delay1) | ||
const p2 = promiseWithDelay(null, delay2) | ||
|
||
const SPOne = defineComponent({ | ||
async serverPrefetch() { | ||
await p1 | ||
}, | ||
render() { | ||
return h(BasicComponentWithUseId) | ||
}, | ||
}) | ||
|
||
const SPTwo = defineComponent({ | ||
async serverPrefetch() { | ||
await p2 | ||
}, | ||
render() { | ||
return h(BasicComponentWithUseId) | ||
}, | ||
}) | ||
|
||
const app = createApp({ | ||
setup() { | ||
const id1 = useId() | ||
const id2 = useId() | ||
return () => [id1, ' ', id2, ' ', h(SPOne), ' ', h(SPTwo)] | ||
}, | ||
}) | ||
return [app, [p1, p2]] | ||
} | ||
|
||
const expected = | ||
'v0:0 v0:1 ' + // root | ||
'v1:0 v1:1 ' + // inside first async subtree | ||
'v2:0 v2:1' // inside second async subtree | ||
// assert different async resolution order does not affect id stable-ness | ||
expect(await getOutput(() => factory(10, 20))).toBe(expected) | ||
expect(await getOutput(() => factory(20, 10))).toBe(expected) | ||
}) | ||
|
||
test('async setup()', async () => { | ||
const factory = ( | ||
delay1: number, | ||
delay2: number, | ||
): ReturnType<TestCaseFactory> => { | ||
const p1 = promiseWithDelay(null, delay1) | ||
const p2 = promiseWithDelay(null, delay2) | ||
|
||
const ASOne = defineComponent({ | ||
async setup() { | ||
await p1 | ||
return {} | ||
}, | ||
render() { | ||
return h(BasicComponentWithUseId) | ||
}, | ||
}) | ||
|
||
const ASTwo = defineComponent({ | ||
async setup() { | ||
await p2 | ||
return {} | ||
}, | ||
render() { | ||
return h(BasicComponentWithUseId) | ||
}, | ||
}) | ||
|
||
const app = createApp({ | ||
setup() { | ||
const id1 = useId() | ||
const id2 = useId() | ||
return () => | ||
h(Suspense, null, { | ||
default: h('div', [id1, ' ', id2, ' ', h(ASOne), ' ', h(ASTwo)]), | ||
}) | ||
}, | ||
}) | ||
return [app, [p1, p2]] | ||
} | ||
|
||
const expected = | ||
'<div>' + | ||
'v0:0 v0:1 ' + // root | ||
'v1:0 v1:1 ' + // inside first async subtree | ||
'v2:0 v2:1' + // inside second async subtree | ||
'</div>' | ||
// assert different async resolution order does not affect id stable-ness | ||
expect(await getOutput(() => factory(10, 20))).toBe(expected) | ||
expect(await getOutput(() => factory(20, 10))).toBe(expected) | ||
}) | ||
}) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
import { | ||
type ComponentInternalInstance, | ||
getCurrentInstance, | ||
} from '../component' | ||
import { warn } from '../warning' | ||
|
||
export function useId() { | ||
const i = getCurrentInstance() | ||
if (i) { | ||
return (i.appContext.config.idPrefix || 'v') + i.ids[0] + ':' + i.ids[1]++ | ||
} else if (__DEV__) { | ||
warn( | ||
`useId() is called when there is no active component ` + | ||
`instance to be associated with.`, | ||
) | ||
} | ||
} | ||
|
||
/** | ||
* There are 3 types of async boundaries: | ||
* - async components | ||
* - components with async setup() | ||
* - components with serverPrefetch | ||
*/ | ||
export function markAsyncBoundary(instance: ComponentInternalInstance) { | ||
instance.ids = [++instance.ids[2], 0, 0] | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yyx990803 is there any good reason to not throw an error in case
i == false
?The code as it is right now results in return type of
string | undefined
.Sure, a user might call
useId
outside a component, but I feel like this should just produce an error.I am having the same problem as @dr46ul describes here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
None of the APIs that require an active currentInstance throws. This avoids breaking the entire user experience in unexpected cases and leaves the option to throw to the developer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd argue that not throwing in a situation where an active currentInstance is required constitutes "silent failure". But then again console warnings are issued and I might have spent too much time in python-land recently to appreciate avoiding a throw. 😂
Thanks for explaining! 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason then it cannot return an empty string, (or some type of string) rather than
undefined
?The
string | undefined
return type causes downstream apps (including Nuxt) to have to do something likeuseId()!
or YOLO castconst myId = useId() as string
to avoid having to account forundefined
even when the dev knows they are calling it properly.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess yolo-casting ist he way to go if you know you are calling it properly? (i.e. inside a component).
I now actually sort of appreciate this Situation since I was calling
useId
inside a composable and handling this situation explicitly (with a throw) prevents missunderstandings like trying to use it in a store. (But there are other composables that could be used in a store).