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

Regression: can-diff/list/ causes can-reflect.getSchema to throw an error #2

Closed
chasenlehara opened this issue Jun 5, 2018 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@chasenlehara
Copy link
Member

[This is arguably a bug in can-reflect, but since can-diff is what changed recently, filing it here.]

This code is new in can-diff:

can-diff/list/list.js

Lines 43 to 45 in bb2fb96

if(typeSchema == null && oldListLength > 0) {
typeSchema = canReflect.getSchema( canReflect.getKeyValue(oldList, 0) );
}

One might expect that if oldListLength > 0, then oldListLength[0] would be present… but this isn’t always the case. 😅

I’m not 100% sure what code is responsible for making the list, but oldList is an array with objects at various indices, not including [0], so that line passes undefined to can-reflect.getSchema, which throws here: https://github.com/canjs/can-reflect/blob/339f55666ce3101482e5d3fc3262a7626ae6e63a/reflections/shape/schema/schema.js#L106

If need be, I’m happy to pair on this to show it in a real project.

@chasenlehara chasenlehara added the bug Something isn't working label Jun 5, 2018
@justinbmeyer
Copy link
Contributor

getSchema should probably return undefined if undefined is passed.

chasenlehara added a commit to canjs/can-reflect that referenced this issue Jul 3, 2018
`canReflect.getSchema( undefined )` now returns `undefined`

Fixes canjs/can-diff#2
@chasenlehara chasenlehara self-assigned this Jul 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants