Skip to content

Add failling test for coerce #1125

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

Closed
wants to merge 1 commit into from
Closed

Add failling test for coerce #1125

wants to merge 1 commit into from

Conversation

rreusser
Copy link
Contributor

@rreusser rreusser commented Nov 9, 2016

This PR adds a failing test for Lib.coerce. It's pretty short:

var attrs = { 
  x: {
    valType: 'info_array',
    freeLength: true,
    items: [
      { valType: 'any' },
      { valType: 'any' },
      { valType: 'any' }
    ]   
  }
};
out = coerce({x: [[], 10]}, {}, attrs, 'x');
expect(out).toEqual([[], 10]);

It fails because the output is equal to [undefined, 10] instead of [[], 10].

The error goes away if I remove the third valType: 'any', suggesting that freeLength doesn't quite do what it sounds like it does. freeLength has no effect either way, but I've included it since I'd expect it to help. It seems it's refusing to set empty arrays, but I'm not sure why.

@rreusser rreusser added status: in progress bug something broken labels Nov 9, 2016
@rreusser
Copy link
Contributor Author

rreusser commented Nov 9, 2016

Found the source of the problem: https://github.com/plotly/plotly.js/blob/master/src/lib/nested_property.js#L243

The solution is not straightforward. The result is that you can't actually pass an empty array through plotly. That makes a pause button a bit awkward.

@rreusser
Copy link
Contributor Author

rreusser commented Nov 9, 2016

Solution: The cause of this is that Lib.nestedProperty is written to aggressively clean up after itself. This touches a lot of things so is best not modified. For a pause button where an affirmative empty array [] is meaningful and does not indicate the item should be deleted, it seems the best solution, contingent upon #1121, is to require [null] for pausing instead of [].

@rreusser rreusser closed this Nov 9, 2016
@etpinard etpinard deleted the pause-button-fix branch November 15, 2016 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant