-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Interactivity API: Prevent each directive errors and allow any iterable #67798
Changes from all commits
5f49181
41593b3
dacc7c6
1aada21
938f852
c04e29b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,7 +18,7 @@ test.describe( 'data-wp-each', () => { | |
await utils.deleteAllPosts(); | ||
} ); | ||
|
||
test( 'should use `item` as the defaul item name in the context', async ( { | ||
test( 'should use `item` as the default item name in the context', async ( { | ||
page, | ||
} ) => { | ||
const elements = page.getByTestId( 'letters' ).getByTestId( 'item' ); | ||
|
@@ -500,4 +500,37 @@ test.describe( 'data-wp-each', () => { | |
await expect( element ).toHaveText( 'beta' ); | ||
await expect( callbackRunCount ).toHaveText( '1' ); | ||
} ); | ||
|
||
for ( const testId of [ | ||
'each-with-unset', | ||
'each-with-null', | ||
'each-with-undefined', | ||
] ) { | ||
test( `does not error with non-iterable values: ${ testId }`, async ( { | ||
page, | ||
} ) => { | ||
await expect( page.getByTestId( testId ) ).toBeEmpty(); | ||
} ); | ||
} | ||
|
||
for ( const [ testId, values ] of [ | ||
[ 'each-with-array', [ 'an', 'array' ] ], | ||
[ 'each-with-set', [ 'a', 'set' ] ], | ||
[ 'each-with-string', [ 's', 't', 'r' ] ], | ||
[ 'each-with-generator', [ 'a', 'generator' ] ], | ||
|
||
// TODO: Is there a problem with proxies here? | ||
// [ 'each-with-iterator', [ 'implements', 'iterator' ] ], | ||
Comment on lines
+522
to
+523
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it still the case that there is a problem with proxies? I see that tests are passing in the CI. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, I realized you meant there's a problem only for the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's right, it's commented out because it would not pass. I didn't spend much time debugging the problem so I'm not sure what the issue is. It's some thing that apparently should work but does not, I'm not sure of the cause and don't really plan to spend time debugging it, maybe @DAreRodz know if this would create issues with store proxies. |
||
] as const ) { | ||
test( `support different each iterable values: ${ testId }`, async ( { | ||
page, | ||
} ) => { | ||
const element = page.getByTestId( testId ); | ||
for ( const value of values ) { | ||
await expect( | ||
element.getByText( value, { exact: true } ) | ||
).toBeVisible(); | ||
} | ||
} ); | ||
} | ||
} ); |
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'll admit, this is a bit esoteric, but it doesn't work in the iAPI but it seems like it could. I suspect its iterator is lost in the proxies.