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.

Sign up for GitHub

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

Jump to bottom

array().ensure() not casting single element #343

Closed
fionawhim opened this issue Oct 16, 2018 · 3 comments · May be fixed by #347
Closed

array().ensure() not casting single element #343

fionawhim opened this issue Oct 16, 2018 · 3 comments · May be fixed by #347
Labels
bug

Comments

@fionawhim
Copy link

The docs for array.ensure say that it will cast a non-array into a single element array:

array()
  .ensure()
  .cast(1); // -> [1]

However in 0.26.6 on Node, the above returns []. It looks like the transform call in the array constructor (run with withMutation) runs first and sets the value to null when it sees it’s not an array. Therefore the transform method in ensure sees null rather than the original value.

I’m happy to make a PR if I could understand what the right solution is for the fix. Are the transforms not being called in the right order?
@fionawhim
Copy link
Author

You can make it work by having the ensure transform operate on the originalValue (second argument) rather than the first. If that's a reasonable fix I can make a PR, otherwise I could modify the docs.

@jquense jquense added the bug label Oct 17, 2018
@jquense
Copy link
Owner

jquense commented Oct 17, 2018

that sounds reasonable, happy to take a fix!

fionawhim pushed a commit to CityOfBoston/yup that referenced this issue Oct 17, 2018
Changes array.ensure to work on the original value
4655a8e 
This fixes to match the documentation, which says that a single element
should get cast to an array containing that element. Previously the
transform added in the constructor would convert non-arrays to null
before the ensure transform function would see them.

Fixes jquense#343

WARNING: This is a breaking change. `ensure` now ignores any
transformations added before it, including the transform in the
constructor that parses JSON.
fionawhim pushed a commit to CityOfBoston/yup that referenced this issue Oct 17, 2018
Changes array.ensure to work on the original value
b129ed3 
This fixes to match the documentation, which says that a single element
should get cast to an array containing that element. Previously the
transform added in the constructor would convert non-arrays to null
before the ensure transform function would see them.

Fixes jquense#343

WARNING: This is a breaking change. `ensure` now ignores any
transformations added before it, including the transform in the
constructor that parses JSON.
@fionawhim fionawhim mentioned this issue Oct 17, 2018
Open
@mwojslaw
Copy link

mwojslaw commented Apr 9, 2019

Hi guys, there is some plan for releasing this fix ?

@jquense jquense closed this as completed in 94659c2 Mar 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Changes array.ensure to work on the original value CityOfBoston/yup
3 participants
@fionawhim @jquense @mwojslaw