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

Remove some schema mapping heuristics? #692

Open
LeaVerou opened this issue Feb 2, 2021 · 13 comments
Open

Remove some schema mapping heuristics? #692

LeaVerou opened this issue Feb 2, 2021 · 13 comments

Comments

@LeaVerou
Copy link
Member

LeaVerou commented Feb 2, 2021

We currently have a lot of schema mapping heuristics, only some of which are documented.

I've made a pass through the code to make a list:

  1. Singletons can be rendered on collections, they are just wrapped in an array
  2. Arrays can be rendered on singletons. If the singleton is not the root object, then the first item is rendered (and the rest are preserved, but not shown). If the singleton is the root object, then the array is rendered:
    1. on the collection with property=main
    2. If none found, on the first writable collection
    3. If none found, the first item is rendered instead, just like any other object.
  3. Superfluous properties are retained, and even reachable from expressions.
  4. Nested objects with only one property that is the same as the parent property are "unwrapped". This means that {foo: {foo: 5}} when rendered on the foo property is the same as rendering {foo: 5}.
  5. Primitives can be rendered on objects.
    1. If the object contains a property with the same name, it is rendered there.
    2. Failing that, the first writable primitive property
    3. FFS, there is even a ...scoring function!
  6. Objects can be rendered on primitives. What is rendered?
    1. If Symbol.toPrimitive exists, it is invoked and the return value is rendered.
    2. If it's a plain object, we render one of its properties on the primitive (preserving the rest):
      1. If there is only one property, we choose that
      2. If there is a property that is the same as the primitive's property, we choose that
      3. If there is a value or content property, we choose that
      4. Failing that, any property with the same datatype.
  7. Some of the rules above are not applied if the node is a helper variable, i.e. a <meta>.

As discussed with @karger, not all of these heuristics are useful, and some end up making Mavo's behavior rather unpredictable. Which ones should we drop, and which ones are useful enough to be worth their complexity?

Consider cases where we are rendering data we don't control (e.g. via an API), or the user doesn't have full sense of the data schema (like the shopping list)

@LeaVerou
Copy link
Member Author

LeaVerou commented Feb 3, 2021

So, at first glance, it looks like most of 5 and 6 needs to go, as those are the most weird and confusing ones. The current behavior of 6 can always be emulated with mv-path. I'm not sure how we'd handle 5 though. @karger?

@DmitrySharabin
Copy link
Member

I definitely had a chance to benefit from the first four in my apps and plugins (1–4).

Speaking about 5, I don't fully understand what it all means, to be honest.

And if 6 can always be emulated with mv-path, I'd personally prefer to be more explicit in the code, so anyone who reads it fully understands (maybe not from the first glance) what's going on in the app.

@LeaVerou
Copy link
Member Author

LeaVerou commented Feb 9, 2021

I started work on removing 6.ii. This means that when e.g. rendering {"foo": 1, "bar": 2} on a Primitive, instead of seeing 1 which could be confusing, we'll see {"foo": 1, "bar": 2}, which helps the author understand what's going on.
Sounds good, right?

Well, as usual, nothing is ever as simple as it first seems.

What happens when this is edited?? Do we disable editing? Do we let the user edit the serialization and then re-parse? If the latter, then what happens when we're not dealing with plain objects, but instances of classes, with functions etc? Do we let the user edit the serialization and then treat it as a string? That seems horrid.

@DmitrySharabin
Copy link
Member

DmitrySharabin commented Feb 9, 2021

Just to be sure that I understand everything correctly, let me think out aloud.

Let's say an external API is used as the data source for a Mavo app. It returns an object with the following structure: {"foo": 1, "bar": 2}. The application has a single property named baz. According to the heuristic described in 6.ii, previously, Mavo would output the value 1. Now it will output {"foo": 1, "bar": 2}, right?

In other words, we have something like that:

<div mv-app mv-source="https://awesome/api">
  <E property="baz"></E>
</div>

And if instead of the baz property, we have the bar property inside the app, we'll get 2, right?

To my mind, by outputting {"foo": 1, "bar": 2} Mavo clearly shows the author that their app's structure doesn't reflect the data structure they get, and they need to perform some extra steps to fix it. E.g., by adding mv-alias to the corresponding property.

Is there a use case where the author needs to edit the serialization and afterward save it? Why do they need it? To massage data somehow so they can be structured the way the author expects? I believe they need it only once, no?

Concerning the expected behavior, I would expect that Mavo will disable editing in these cases and will indicate it somehow.

@karger
Copy link
Collaborator

karger commented Feb 10, 2021

For 6 my first instinct is to agree with @DmitrySharabin . If we have a schema mismatch, we should show the actual data and disable editing. If we want to be fancier, for the case where an object is being rendered in a primitive, we can create and invoke an "object editor" for example the right pane of https://jsoneditoronline.org/ which lets you mutate the structure and modify the primitive values.

For 5, it seems to me that what should happen is so ambiguous that I'd be happy just not rendering anything. You frame it in terms of uncontrollable APIs, but can't this happen just if e.g. some mavo function returns an object whose schema doesn't match the html template on which it is rendered?

    <div property="foo" mv-value="group(a: 1)">
        <span property="b"></span>
   </div>

I'd say now foo={a:1, b:null} and nothing should show in the b span.

I'd also like to consider an alternative heuristic for 2: if you render an array on the singleton, the singleton gets replicated as if it's an mv-multiple. I think this would have saved me much debugging and it seems like it might often be exactly what the user wants.

@LeaVerou
Copy link
Member Author

For 6 my first instinct is to agree with @DmitrySharabin . If we have a schema mismatch, we should show the actual data and disable editing. If we want to be fancier, for the case where an object is being rendered in a primitive, we can create and invoke an "object editor" for example the right pane of jsoneditoronline.org which lets you mutate the structure and modify the primitive values.

We definitely don't want to be fancier. Implementing an object editor would take a lot of resources, and wouldn't address all cases (e.g. your object may not be JSON serializable, it could be an instance of a class, with functions etc)

For 5, it seems to me that what should happen is so ambiguous that I'd be happy just not rendering anything. You frame it in terms of uncontrollable APIs, but can't this happen just if e.g. some mavo function returns an object whose schema doesn't match the html template on which it is rendered?

    <div property="foo" mv-value="group(a: 1)">
        <span property="b"></span>
   </div>

I'd say now foo={a:1, b:null} and nothing should show in the b span.

Indeed nothing will show in the b span, even before this change. But this doesn't make the b span uneditable, nor should it.

I'd also like to consider an alternative heuristic for 2: if you render an array on the singleton, the singleton gets replicated as if it's an mv-multiple. I think this would have saved me much debugging and it seems like it might often be exactly what the user wants.

We decided not to do that early on though I don't remember the exact reasoning. I have certainly made this mistake as well. The use case of only showing one item from a list on purpose is certainly quite rare.
Implementing this would also be quite hard, you'd basically need to be able to convert anything to a collection and back at any point. Also what happens with editing? Is that collection editable?

LeaVerou added a commit that referenced this issue Feb 11, 2021
Instead of looking for a property to render and creating an implicit mv-path we now show a serialization of the object and temporarily disable editing.
@LeaVerou
Copy link
Member Author

LeaVerou commented Jul 5, 2021

What should we do about 2? We should definitely be able to render singletons on arrays (it's common to turn an existing node into a list once needs change), but do we need the opposite? (rendering arrays on singletons by rendering the first item)

The latter behavior can be sort of emulated with mv-path="0" or mv-alias="0" (though those will only work if every value is an array), and having this heuristic in place means it's impossible to distinguish between a helper variable on which we want to render an entire array, or a previous collection that is now a singleton. It also means that on primitives, node.value = foo behaves very differently than node.render(foo).

On the other hand, if we just accept the array, then we can end up with arrays or arrays and things like that.

Thoughts?

@karger
Copy link
Collaborator

karger commented Jul 5, 2021 via email

@LeaVerou
Copy link
Member Author

LeaVerou commented Jul 6, 2021

@karger I did not understand what you mean?

@davemo
Copy link

davemo commented Nov 30, 2021

Jumping in here on the invitation of @DmitrySharabin in #797, some of the implicit schema mapping (and rendering) definitely confused me as a first-time Mavo user. (specifically point 6 above).

Here's my thought process and how I approached troubleshooting/learning Mavo:

  1. I read through the docs on mavo.io/docs (which are pretty good!)
  2. When I hit an issue with behaviour that surprised me (implicit rendering of the JSON object in the DOM element), I re-read the sections on Collections to see if there might have been some clues
  3. Then I went through the index of all attributes/classes
  4. I then spent time step-debugging through some of the internals of primitive.js

I think where I went wrong was this sentence describing mv-group in the docs, which I missed entirely during my first read:

This is almost always inferred from the structure of your properties and added automatically, but you can use it to force an element to be a group.

The nature of "almost always inferred" definitely seems like it could create ambiguity and complexity in the underlying implementation, but I saw that and missed the last part of the sentence about manually forcing the group structure in markup.

That said, I'm happy to understand now and I started wondering about ways to improve either the docs or the behaviour (or perhaps both). One other thing I found somewhat puzzling was that the docs make reference to both 0.3.0 and 0.2.4 releases, but I could only find code for the latter.

Is there an upcoming new release? 🤔

@DmitrySharabin
Copy link
Member

DmitrySharabin commented Nov 30, 2021

Is there an upcoming new release? 🤔

Yes. The current dev version (the one you are experimenting with) is supposed to be the next v0.3.0 release.

@davemo
Copy link

davemo commented Nov 30, 2021

Ah, this confused me because I saw the value of Mavo.version as v0.2.4 in the console, but the docs seemed to indicate v0.3.0 was already released:

Screen Shot 2021-11-30 at 7 17 06 AM

I also cloned the repo and ran the build script just to see if the primary branch had new changes but it still reported v0.2.4; I suppose this makes sense if you haven't yet released the new version. 🤔

@LeaVerou
Copy link
Member Author

LeaVerou commented Nov 30, 2021

Yeah, that is indeed confusing and we should fix it, thanks @davemo!

  1. Mavo.version should be v0.3.0beta or v0.3.0dev or something.
  2. Same in package.json

However, given that most people won't look in either of these places, there should be something about v0.3.0 in https://mavo.io/get/ (ideally not hardcoded but automatically updating with whatever the next version will be)

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

4 participants