pattern match failure, crossed-wire?

[MichaelXavier]

Hi,

I'm experimenting with pux in a public project. Its just pretty much a pux app with a bunch of small demos of increasing complexity. One of them is an AJAX-backed list. I have a component for the AJAXList and a child component for the AJAXListItem, and the Action for AJAXList is partly composed of child actions for AJAXListItem, which seems pretty standard. Interestingly, in the AJAX list case, firing an item's Delete action should get wrapped in the parent's ItemAction constructor, but it actually results in a pattern match failure. It almost seems like the wrong signal is being provided to the wrong component.

Here's how you'd repro it:

git clone https://github.com/MichaelXavier/pux-scrapyard.git --branch rest
cd pux-scrapyard
npm install
bower install
npm run-script build
npm run-script server

Browse to http://localhost:8000 and click on AJAX List, then open your dev console and click Delete by the first item. You'll see the following error in the console:

index.js:25374 Uncaught Error: Failed pattern match at Components.AJAXList line 60, column 1 - line 61, column 45: DeleteItem,Object

The function in question is expecting to receive ItemAction Int AJAXListItem.Action but is instead receiving AJAXListItem.Action, despite the types check passing.

Interestingly, there's also a demo there for a non-effectful list/listitem component that works fine. This leads me to believe that something about the effects system may be causing a signal to get routed to the wrong component or perhaps its dropping the wrapped version of the signal it uses for the parent's update function.

Sorry for the somewhat convoluted repro steps. Please let me know if there's any further info I can provide!

[MichaelXavier]

I traced the callstack a bit and threw in some debuggers and it looks like at the top level the value being passed in is:

AppAction {value0: AJAXListAction {value0: DeleteItem}}

Which would not typecheck, because the code is like

data AppAction = AJAXListAction AJAXListItem
data AJAXListAction = ItemAction Int AJAXListItemAction
data AJAXListItemAction = DeleteItem

So a layer is being stripped somewhere. I haven't tracked that down because I think that happens in Signal land and I think that may take it out of the call stack that the error gives me.

[MichaelXavier]

So I went on a QA safari here and started fiddling with the code to see when this bug manifests. Its concerning because AJAXList is basically the same exact architecture but just doing effects. Here's the crazy part: this pattern match error stops happening if I reduce AJAXListItem.Action to a single constructor, a type alias or a newtype. Check out this diff:

https://github.com/MichaelXavier/pux-scrapyard/compare/rest...single-action-ajax?expand=1

That branch actually works. I have no idea what is going on. Is this a purescript bug? Is this a browserify/pulp bug? It persists with or without the --optimise flag on pulp build. The even weirder thing is that the List component has multiple actions, not just 1 and it works....

[MichaelXavier]

Apologies for this thread turning into the rambling of a mad man. At this point I'm actually diffing the browserified output. The attached screenshot of the diff is the only difference between an output that works (no pattern match failure, actions process correctly) and doesn't (pattern match failure exception).

Source on the left hits the pattern match error, source on the right works. The inner code is identical and the only difference is that in the case of 2+ constructors, purescript puts in a pattern match failure exception. Interestingly, in both sources when stepping through with a debugger, v is the same, DeleteItem {}, so its actually the wrong type in both code samples and is ony causing an issue due to being caught. Oddly enough though, the code on the left works, so its like there's still a bug at play but it doesn't actually break functionality. I think this has something to do with composeAction in pux but I haven't been able to track it down, but it seems like its losing/erasing an intermediate constructor.

I actually thought I'd try upgrading from psc 0.9.3 to the new 0.10.1 but the ecosystem hasn't caught up yet to 0.10 and pux won't compile under it.

[charleso]

@MichaelXavier This is a wild guess, but I think you're looking in the wrong place. It's got nothing to do with the pattern match, it has to do with the lack of the Html Functor working here

My guess is that after using the bind instance, which is really just append, you will end up with a non-functor supporting Html element. This in turn will mean that the map function won't work. I suspect you're hitting this line and stopping, which means you'll only have a DeleteItem value, not ItemAction:

purescript-pux/src/Pux/Html/Elements.js

Line 28 in 537f15c

if (!html.props) return html

Basically, what happens if you don't use do here, and construct a proper parent node (eg. div or something like that)?

https://github.com/MichaelXavier/pux-scrapyard/blob/rest/src/Components/AJAXListItem.purs#L71

[charleso]

Honestly, the append/bind function for Html looks pretty broken to me in general. I wouldn't use it. I may create a (I suspect controversial) PR in a few hours to remove it.

[MichaelXavier]

@charleso interesting! Thanks so much for checking this out. I'll look into this after work tonight. I have been fiddling around with this in a branch and actually tore down this module and built it back up almost identically and its working now. I have even odds that it was either something cached incorrectly in bower or the issue you outlined. I'll try this out on a commit I know that exhibits this.

Out of curiosity, are you saying that the Bind instance for Html is the suspect? Is it possible this instance isn't law-abiding? If that's the case, if its not law-abiding and breaks code that type checks I'll happily chime in on your PR and support it. That would be a pretty damning case against it. I don't think I've done anything too obviously crazy with my code and to have the instance produce such an obscure bug is a pretty big problem IMO.

[charleso]

@MichaelXavier #72

[charleso]

@MichaelXavier Technically it's not a monad. TIL you can rebind do's purescript/purescript#962. I'm not sure it's a good idea, seems to make things harder to read, do is no longer just Monad.

[kritzcreek]

🕵️ charleso - great work!

[mostalive]

@charleso thanks for figuring this out, I'm getting pattern match failures occasionally. I was already in the process of removing 'do', syntax with arrays is easier to understand for my colleague who doesn't do full purescript just yet.

[MichaelXavier]

FYI I think dropping the monad instance fixed it.