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

Missing attributes during conversion using an ActionFunction with morphism() #52

Closed
terlar opened this issue Feb 23, 2019 · 11 comments
Closed
Assignees

Comments

@terlar
Copy link

terlar commented Feb 23, 2019

If I have a definition such as:

interface Grade = {
  level: 'A'|'B'|'C'
  note?: string
}
interface Student = {
  name: string
  grade?: Grade
}

interface StudentDocument = {
  name: string,
  grade_data?: {
    level: 'A'|'B'|'C'
    note?: string
  }
}

const gradeSchema: StrictSchema<Grade> = {
  level: 'level',
  note: 'note'
}
const studentSchema: StrictSchema<Student, StudentDocument> = {
  name: 'name',
  grade: ({ grade_data }: any) => morphism(gradeSchema, grade_data),
}

Then the following function call will give:

morphism(studentSchema, { name: 'John Doe' })
{ name: 'John Doe',
  grade: [Function] }

I can wrap this in something like:

grade: ({ grade_data }: any) => grade_data ? morphism(gradeSchema, grade_data) : null

However it would be nice if the morphism never resulted in a function even with missing values. What is the strategy when working with optional values?

@emyann
Copy link
Member

emyann commented Feb 23, 2019

Hello @terlar,

Thank you for the feedback.

By design Morphism is a currying function emitting either the final result, or a mapping function aimed to be reused later.

Without any source, keep the mapper to be reused later

const mapper = morphism(schema)
// mapper(source) => final result

Straight map when a schema and a source are provided

const result = morphism(schema, source)

If you are not sure of the value of grade_data when you are doing some nested mapping you have the good approach with:

grade: ({ grade_data }) => grade_data ? morphism(gradeSchema, grade_data) : null

Also this brings the opportunity for you to normalize the data you want to expose.

You can force the mapping by spreading the input: https://stackblitz.com/edit/typescript-nxc4hj

grade: ({ grade_data }) => morphism(gradeSchema, {...grade_data})

morphism(studentSchema, { name: 'John Doe'})
// ==> { name: 'John Doe', grade: { level: undefined, note: undefined } }

What would you expect as a return value if morphism(gradeSchema, grade_data) didn't fallback on the mapper function ?

@emyann emyann self-assigned this Feb 24, 2019
@terlar
Copy link
Author

terlar commented Feb 24, 2019

I think if the type declaration is optional it should act as if the value was not provided and omit the key completely, e.g.:

> interface MyTest {
... aProperty: string
... bProperty?: string
... }
undefined
> let a: MyTest = { aProperty: 'hello' }
undefined
> a
{ aProperty: 'hello' }

However it would be nice if you could enforce the types if they are not optional, e.g.:

> let a: MyTest = { }
[eval].ts(5,5): error TS2322: Type '{}' is not assignable to type 'MyTest'.
  Property 'aProperty' is missing in type '{}'.

undefined

In the cases of non-optional types I do agree that there is not default fallback and should probably error out instead, or force the transformation to specify how to handle that.

The problem with the functions although i understand from an implementation behavior how that is neat. But in the final transformation if you did some mistake or whatever, a function is never the correct value? At least not when working with plain data.

@emyann
Copy link
Member

emyann commented Feb 25, 2019

I think if the type declaration is optional it should act as if the value was not provided and omit the key completely

The problem with the functions although i understand from an implementation behavior how that is neat.

Yes I do agree with this one. I won't be able to know at runtime if the field is optional from TypeScript , but I can definitely omit the key if the transformation returns an undefined value. I will create a PR for that 👍

However it would be nice if you could enforce the types if they are not optional

Do you use the last version of Morphism ? Some improvements has been made on TypeScript types to help this kind of scenario. In v2 I am planning to let the user opt in for runtime warning when a schema is not met (e.g: a transformation returning an undefined value)

@terlar
Copy link
Author

terlar commented Feb 25, 2019

I installed it via NPM this weekend, so it should have been the latest published at that point. I saw your road-map and it looks promising. Looking forward to use this more, all really good ideas. I think this has the best TypeScript integration what I have seen. I have been running into things complaining due to missmatching types which is good. Perhaps add the possibility to apply some transformation on the final result. I know I can do it on the result from morphism, but when you are working with arrays it would be nice to apply such a function to all entries without manually using map. My only current use case though is to be able to strip undefined keys (that was optional).

@emyann
Copy link
Member

emyann commented Feb 25, 2019

Thank you so much for the kind words and the feedback 🙏🏽

Perhaps add the possibility to apply some transformation on the final result. I know I can do it on the result from morphism, but when you are working with arrays it would be nice to apply such a function to all entries without manually using map

I am not sure I understood this correctly but you actually have the ability to use Morphism on Objects and Arrays, the rule is it will always respect the input dimension:

e.g.

morphism(schema,{}) => {} // Apply the transformation on the input object

morphism(schema, [{},{}]) => [{},{}] // Apply the transformation on the input array

// or 
[].map(morphism(schema))

I will work on stripping the undefined keys starting next week I think, thank you for the suggestion! :)

@terlar
Copy link
Author

terlar commented Feb 25, 2019

You are welcome.

Yes, you understood me partly, sorry for being a bit unclear. I see now that it was just a blob of text and not very structured.

That is what I meant, since it supports both arrays and direct objects. If you want to apply some function to the transformation you have to be aware of the result being worked on is an array or not and if it is an array you have to manually map as shown. My thought was what the procedure was if you always wanted some post-processing of your morphism transformation, per object level.

I am not sure there is a strong use case, but such as thing as removing null values, etc. could be a thing for the post processing step.

@emyann
Copy link
Member

emyann commented Mar 21, 2019

@terlar I made a little fix to avoid getting a function instead of a undefined property in the case of mapping a undefined data source https://repl.it/@yrnd1/Morphism-undefined-data-source This fix is available with version 1.9.2

I will add some options along the schema in order to apply some strategies on null and/or undefined values. I will think about your suggestion on some processing steps as well.

@terlar
Copy link
Author

terlar commented Mar 22, 2019

@emyann Nice! Thanks, this made me be able to replace a pattern where I had a morphismOrNull function that wrapped this, returning null if there was no value otherwise apply the morphism function. Only difference is that I returned null instead of undefined.

In this case I would still prefer to have the key omitted, but I guess I could have some manual post-processing. So would appreciate some options like that, let me know and I will test it :)

@emyann
Copy link
Member

emyann commented Mar 23, 2019

@terlar Thank you for your feedback !I did release a new version 1.10 with which it is possible to either strip the key when the value is undefined or provide a default value to replace undefined ones.

To do so it is now possible to create a schema with options using the createSchema function.

Strip undefined value

import { createSchema, morphism } from 'morphism'

const schema = createSchema<Target>({ keyA: 'somepath' }, { undefinedValues: { strip: true } })
// => {}

Default value fallback

import { createSchema, morphism } from 'morphism'

const schema = createSchema(
  { keyA: 'somepath' },
  {
    undefinedValues: {
      strip: true,
      default: () => null
    }
  }
);
// => { keyA: null }

Here's a playground if you want to see it working: https://repl.it/@yrnd1/Morphism-undefined-values

Please let me know what you think about it and don't hesitate if you have questions :)

@emyann emyann closed this as completed Apr 7, 2019
@terlar
Copy link
Author

terlar commented May 23, 2019

Sorry for the delay, I didn't have time to update my implementation to use this until now. Just want to return here to say thank you and I am very satisfied with the current behavior. Keep up the good work 👍

@emyann
Copy link
Member

emyann commented May 23, 2019

I really appreciate your feedback @terlar! Thank you and let me know if you need further support :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants