barneycarroll / patchinko

A terse API for performing deep patching on JavaScript structures
MIT License
73 stars 5 forks source link

Detect functions and implicitly wrap in `O`? #30

Open JAForbes opened 5 years ago

JAForbes commented 5 years ago

Proposal


// OLD:
O({ layoutEnabled: O( x => !x ), loggedIn: false })

// NEW:
O({ layoutEnabled: x => !x, loggedIn: false })

The primary use case of O is for state management not monkey patching, and it would be convenient to not need to manually wrap functions in O when detecting that a value is a function is 95% likely to be intended to be a patching function for that key instead.

This would enable patterns like meiosis to have one call site for patchinko in the entire application, and component code need not know how their patch is being processed as they don't need to import patchinko to do a patch on a key'd value.

Background

@barneycarroll we've discussed this before I think. The original reason you wrote patchinko was to monkey patch mutably an existing API.

It seems to me, the immutable version of patchinko has a completely different value proposition. It's an easy to use reducer in the context of meiosis/actor model style apps. And those state objects that are being reduced rarely store functions within the state tree (I think we could call that an anti pattern).

Normally I'm not in favour of implicit behaviour, but that's kind of the value proposition of O so I figure why not complete the circle.

What do you think?

JAForbes commented 5 years ago

Tieing in with #27 I support additionally having an O( target => target ) which is actually a query

barneycarroll commented 5 years ago

Old use case still achievable with this proposal:

// OLD:
O(target, { layoutEnabled: modelFunction })
// NEW:
O(target, { layoutEnabled: () => modelFunction })

But then I wonder about implicit recursive interpretation by default:

// legit
O(target, { layoutEnabled: x => x && O}) // < conditionally delete the property
O(target, { layoutEnabled: () => x => !x}) // < also interpretting return value as part of the loop?

...Which suggests the entire O patch input is an implicitly recursive magical object, and it's impossible to supply a random user-land entity without first vetting it as Patchinko-compatible, ie do any aspects of its shape equate to hints that will determine functionality.

Worth looking at @fuzetsu's Mergerino for this kind of level of magic. There we have a different attitude: merges supplied to the top-level consuming operation are flattened, meaning direct input has to be conscious on some level (or we just assume it's hashes all the way down - fair enough)... But the scope function (SUB) also stands in as a directive - when supplied as a value - to skip recursive interpretation. With Patchinko the philosophy is very much to have recursive interpretation made explicit via Patchinko API entities.

I'm intrigued by the concept of structures that defer all interpretation to the top level call site - didn't you @JAForbes once suggest a virtual DOM component API where the hyperscript function was itself supplied to the component function? - inasmuch as we get a kind of dependency-injection based architecture where you eschew constant API imports... But this does a feel like a higher order of magical to me.

What say you?

JAForbes commented 5 years ago

it's impossible to supply a random user-land entity without first vetting it as Patchinko-compatible

I'm not following this point. Are you saying this makes it genuinely ambiguous to Patchinko in ways that were non recursive before?

Didn't you have to sniff the return value of O() previously to detect patchinko operations like O?

With Patchinko the philosophy is very much to have recursive interpretation made explicit via Patchinko API entities.

I really like the explicit operators, but it seems like O is meant to be implicit, so does the same philosophy apply?

I'm intrigued by the concept of structures that defer all interpretation to the top level call site - didn't you @JAForbes once suggest a virtual DOM component API where the hyperscript function was itself supplied to the component function?

I did! 😀 that was to enable return values from event handlers to be interpreted as actions without manually dispatching (Like Elm). I still think that's a good idea, but I'm also wary of deviating too far from mithril's standard hyperscript without it being statically resolved so I can undo it if it turns out to be a bad idea :D. Which is the negative aspect of DI.

But this does a feel like a higher order of magical to me.

Yeah, it is. The main reason I want this isn't so much about a single import, even though that is very nice to have (hello mithril!). It's more compression/information theory.

If everytime I use a function when patching via patchinko's O my intent is always the same, then wrapping the fn in O is not information. If it is always there, it need not not be there.

You could say, it's noise, not signal. If I refer to the docs:

useful when the essential patching operations are intuitive but the different API invocations are cognitively overbearing to determine or noisy to read.

(Emphasis mine).

So I interpreted your design intention for O to be pure signal. What if I removed one word from the quote above.

useful when the essential patching operations are intuitive but the different API invocations are cognitively overbearing to determine or noisy to read.

If we're avoiding some API invocations to reduce noise, why not clarify that and remove all API invocations?

Also given your point:

Old use case still achievable with this proposal:

We're not sacrificing that initial use case, just optimizing the API for a likely more common case.


This all rests on the assumption monkey patching is an exotic use case, maybe that's not true, that's the basis of my entire argument.

foxdonut commented 5 years ago

It looks like we were thinking the same thing at the same time!

https://github.com/fuzetsu/mergerino/pull/4

fuzetsu commented 5 years ago

I just published mergerino 0.3.0 (thanks again @foxdonut)

I took the magic a bit further. function is automatically interpreted as a scope function and undefined as a delete operation. Bypass the behavior for both using SUB.

@barneycarroll

But then I wonder about implicit recursive interpretation by default

This is a very interesting point. Currently mergerino does not try to interpret the return from a scope function either, but I've considered many times whether it should 🤔

Like you say, for conditionally deleting, or merging into an object from a scope function.

O(target, { deep: O(x => O({ layoutEnabled: !x.other })) })

I think @JAForbes has a point regarding the primary use of O being for state management rather than monkey patching. At least the immutable version. That said, introducing significantly different behavior across the flavors of patchinko could get confusing.

JAForbes commented 5 years ago

Great work @fuzetsu I'll give it a try. And your reply to @barneycarroll helped me realise what his concerns were. Yeah I wasn't imagining returning scope being supported, that definitely seems like sugar gone too far.