pzavolinsky / elmx

A tiny precompiler that takes an Elm program with embedded HTML and desugars the HTML into elm-html syntax. Elmx is to Elm what React's JSX is to Javascript
MIT License
351 stars 11 forks source link

Left pipe symbol (<|) produces invalid Elm #7

Closed bbugh closed 8 years ago

bbugh commented 8 years ago

The <| operator is being seen as an open tag by htmlparser2.

Here's a contrived example:

update : Msg -> Model -> (Model, Cmd Msg)
update msg model =
  case msg of
    NoOp ->
      ( model, Cmd.batch <| [Cmd.none] )

subscriptions : Model -> Sub Msg
subscriptions model =
  Sub.none

results in this code:

update : Msg -> Model -> (Model, Cmd Msg)
update msg model =
  case msg of
    NoOp ->
      ( model, Cmd.batch Html.| [, , , , , ] [Html.text " Sub Msg
subscriptions model =
  Sub.none

Which is definitely not valid Elm. 😄

I checked into the elmx parser, and as expected, the htmlparser2 is seeing the pipe symbol as a tag.

# parser.js
    onopentag: function onopentag(name, attrs) {
      // name === "|"

I tried various things to skip operation if the name was a pipe but everything I tried resulted in an error:

TypeError: Cannot read property 'expr' of undefined
    at generate (/Users/bbugh/projects/elm/whaticaneat.com/node_modules/elmx/dist/generator.js:83:18)

It looks like since htmlparser2 reports an open tag, it never properly reports a closed tag, so everything comes through as text mode. I'm not familiar with that library and other than specifying a custom tokenizer, I didn't see any way to tell it to skip certain tags.

I fixed it temporarily for myself by simply replacing <| with ⭐️ before the tokenization, and then swapping them back after the generation. I'm sure there's a better way, though. 😆

pzavolinsky commented 8 years ago

Hey @bbugh thanks for tracking this bug!

I fixed it temporarily for myself by simply replacing <| with :star: before the tokenization, and then swapping them back after the generation.

I've just pushed a fix which is basically what you suggested.

I'm sure there's a better way, though.

The htmlparser2 docs are sparse at best and I couldn't find anything that would work in this scenario. In any case, I like the hack-and-slash solution of the find and replace, not the most elegant one, but definitely the most cost-effective.

I also updated the Atom plugin.

Cheers!