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

The auto-generated outgoingPort JavaScript neglects to escape a record field named "default" #1510

Closed
billstclair opened this issue Oct 21, 2016 · 3 comments
Labels

Comments

@billstclair
Copy link

billstclair commented Oct 21, 2016

Elm version 0.17.1
macOS Sierra version 10.12
Opera 40.0.2308.81

If I build an application that uses an outgoing port to save its state, after the pattern of the elm-todomvc application, and its model contains a record with a field named "default", then the resulting JavaScript code will error as soon as it attempts to reload saved state.

I built a full working Elm project for this and pushed it to https://github.com/billstclair/elm-default-in-port-record. Same Elm code, but an index.html that lets you run it easily.

If you save the file in the next comment as default-in-port-record.elm, and compile it with:

elm make default-in-port-record.elm --output index.js

You can find the following code in the resulting index.js.

default is properly escaped with a prefixed "$" in the compilation of the update function:

var _user$project$Main$update = F2(
    function (msg, model) {
        var _p0 = msg;
        return {
            ctor: '_Tuple2',
            _0: _elm_lang$core$Native_Utils.update(
                model,
                {$default: model.$default + 1}),
            _1: _elm_lang$core$Platform_Cmd$none
        };
    });

But it is NOT escaped in the code that converts the Elm data structures into a structure suitable for JSON encoding for localStorage.setItem():

var _user$project$Main$setStorage = _elm_lang$core$Native_Platform.outgoingPort(
    'setStorage',
    function (v) {
        return {default: v.default};
    });

I looked for where this is done, but am not familiar enough with the compiler to find it.

@process-bot
Copy link

Thanks for the issue! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

@billstclair
Copy link
Author

billstclair commented Oct 21, 2016

I couldn't attach the .elm file, so here it is:


port module Main exposing (..)

import Html exposing (Html, div, text, br, button)
import Html.App as App
import Html.Events exposing (onClick)

import Debug exposing (log)

type alias Model =
  { default : Int
  }

type Msg =
  Increment

main : Program (Maybe Model)
main =
  App.programWithFlags
    { init = init
    , view = view
    , update = updateWithStorage
    , subscriptions = subscriptions
    }

port setStorage : Model -> Cmd msg

-- Copied verbatim from https://github.com/evancz/elm-todomvc/blob/master/Todo.elm
updateWithStorage : Msg -> Model -> ( Model, Cmd Msg )
updateWithStorage msg model =
  let
    ( newModel, cmds ) =
      update msg model
  in
      ( newModel
      , Cmd.batch [ setStorage newModel, cmds ]
      )

model : Model
model =
  { default = 1 }

init : Maybe Model -> ( Model, Cmd Msg )
init savedModel =
  Maybe.withDefault model savedModel ! [ Cmd.none ]

update : Msg -> Model -> ( Model, Cmd Msg )
update msg model =
  case msg of
      Increment ->
        ( { model | default = (model.default + 1) }
        , Cmd.none)

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

view : Model -> Html Msg
view model =
  div []
    [ text ("default: " ++ (toString model.default))
    , br [][]
    , button [ onClick Increment ]
       [ text "Click Me" ]
    ]

@evancz
Copy link
Member

evancz commented Mar 7, 2018

Tracking in #1685 now. Thank you for the report!

@evancz evancz closed this as completed Mar 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants