dillonkearns / elm-typescript-interop

Generate TypeScript declaration files for your elm ports!
BSD 3-Clause "New" or "Revised" License
165 stars 13 forks source link

Comments break compilation #3

Closed muelli closed 6 years ago

muelli commented 6 years ago

could it be that the interop program trips over compiler warnings?

I get this failure:

➜ webapp git:(elm) ✗>~/yarn-v1.3.2/bin/yarn run build
yarn run v1.3.2
$ elm-typescript-interop assets/elm/Main.elm  --output=assets/elm/Main.d.ts  &&  webpack --config webpack.config.js --progress --colors --display-error-details -d --devtool=inline-source-map
Error parsing input file assets/elm/Main.elm

Err ((),{ data = "port module Main exposing (..)\n\nimport Commands exposing (fetchplayers, fetchUser)\nimport Models exposing (Model, initialModel)\nimport Msgs exposing (Msg....\n", position = 338 },["expected end of input"])
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

However, it compiles, although with a warning:

➜ webapp git:(elm) ✗>~/yarn-v1.3.2/bin/yarn run watch
yarn run v1.3.2
$ webpack --config webpack.config.js --progress --colors --display-error-details -d --devtool=inline-source-map --watch
  0% compiling
Webpack is watching the files…

 12% building modules 18/19 modules 1 active ...doweb-elts/webapp/assets/elm/Main.elmRunning elm-make /tmp/webapp-elts/webapp/assets/elm/Main.elm --yes --warn --debug --output /tmp/1171110-9940-8qlhsb.53xag4lsor.js
=================================== WARNINGS ===================================

-- unused import ------------------- /tmp/webapp-elts/webapp/assets/elm/Main.elm

Module `Commands` is unused.

3| import Commands exposing (fetchplayers, fetchUser)
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Best to remove it. Don't save code quality for later!

Success! Compiled 1 module.
Successfully generated /tmp/1171110-9940-8qlhsb.53xag4lsor.js

Hash: df272bc7cb54ce56a3aa                  
Version: webpack 3.10.0
Time: 5202ms
            Asset     Size  Chunks                    Chunk Names
player_search.js  3.02 MB       0  [emitted]  [big]  player_search
   new_player.js  1.77 MB       1  [emitted]  [big]  new_player
          main.js   331 kB       2  [emitted]  [big]  main
   [0] (webpack)/buildin/global.js 509 bytes {0} {1} {2} [built]
   [8] ./assets/ts/playerconsts.ts 270 bytes {0} {1} {2} [built]
  [10] (webpack)/buildin/module.js 517 bytes {0} {1} [built]
  [11] ./assets/ts/app.ts 745 bytes {2} [built]
  [12] ./playercenter/static/playercenter/ts/player_search.ts 4.66 kB {0} [built]
  [13] ./assets/ts/request.ts 3.08 kB {0} [built]
  [17] ./assets/elm/Main.elm 405 kB {0} [built]
  [18] ./playercenter/static/playercenter/ts/new_player.ts 2.2 kB {1} [built]
    + 11 hidden modules
 10% building modules 0/1 modules 1 active ...doweb-elts/webapp/assets/elm/Main.elmStarted compiling Elm..
Running elm-make /tmp/webapp-elts/webapp/assets/elm/Main.elm --yes --warn --debug --output /tmp/1171110-9940-qze58y.m7b4deu3di.js
=================================== WARNINGS ===================================

-- unused import ------------------- /tmp/webapp-elts/webapp/assets/elm/Main.elm

Module `Commands` is unused.

3| import Commands exposing (fetchplayers, fetchUser)
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Best to remove it. Don't save code quality for later!

Success! Compiled 1 module.
Successfully generated /tmp/1171110-9940-qze58y.m7b4deu3di.js

Hash: 31b8286037d8b751ec86                
Version: webpack 3.10.0
Time: 1852ms
dillonkearns commented 6 years ago

Hello @muelli, thanks for reporting the issue. Could you try pasting the source code of your Main.elm file here and tell me if you see any parse errors? http://bogdanp.github.io/elm-ast/example/

If there are errors there, then the issue is with the underlying elm-ast package.

dillonkearns commented 6 years ago

Regardless, that error message should be more clear.

muelli commented 6 years ago

I've pasted

port module Main exposing (..)

init location =
    let
        currentRoute =
            Routing.parseLocation location
    in
        --( initialModel currentRoute, fetchUser )
        ( initialModel currentRoute, Cmd.batch
                                            [ check "yo"
                                            , fetchUser
                                            ]  )

into http://bogdanp.github.io/elm-ast/example/ and it fails:

Err ((),{ data = "port module Main exposing (..)\n\n\ninit location =\n let\n currentRoute =\n Routing.parseLocation location\n in\n --( initialModel currentRoute, fetchUser )\n ( initialModel currentRoute, Cmd.batch\n [ check \"yo\"\n , fetchUser\n ] )\n\n", input = "init location =\n let\n currentRoute =\n Routing.parseLocation location\n in\n --( initialModel currentRoute, fetchUser )\n ( initialModel currentRoute, Cmd.batch\n [ check \"yo\"\n , fetchUser\n ] )\n\n", position = 33 },["expected end of input"])

I believe it's proper Elm though. When removing the comment, it works better.

dillonkearns commented 6 years ago

@muelli yes, I've seen proper elm fail to parse in that package before. Would you mind opening an issue for the https://github.com/Bogdanp/elm-ast package?

I'll leave this issue open to track making the error messaging more informative in elm-typescript-interop when the parsing fails.

dillonkearns commented 6 years ago

Just to update this thread, it looks like a workaround for this issue will be included in the next major version of elm-ast: https://github.com/Bogdanp/elm-ast/issues/41#issuecomment-357274573.

dillonkearns commented 6 years ago

I'm working on moving over to https://github.com/tunguski/elm-ast which seems to fix this issue. However, I'm not sure what the status is of that package, so we may not be able to rely on that for too long. I opened an issue to get a sense: https://github.com/tunguski/elm-ast/issues/33.

I'll post back here when I publish a fix with comment parsing working!

dillonkearns commented 6 years ago

Hello! I pushed a fix for this @muelli. See the changelog for npm version 0.0.5.

Note that I still need to make a couple of changes to get this parsing Elm 0.19 program types properly. If you're still on 0.18, would you mind checking how this works with your project now? This particular issue should be resolved, but I'd love your overall feedback.

Thank you!