WIP: Improve port errors

[rtfeldman]

NOTE: I have not actually tested this out locally yet, but wanted to get feedback on whether this is a good general direction before making it mergeable.

Motivation

I called Elm.embed(Elm.Foo) and got the unhelpful error Uncaught TypeError: Cannot read property 'make' of undefined

It turned out the problem was that I had copy/pasted the contents of Foo.elm to get up and running, and forgot to change its first line to module Foo where - so there was no such thing as Elm.Foo in JS land.

Since this is an easy thing to mess up, a more helpful error message would have pointed me in the right direction.

Solution

This PR does two things:

1. Introduces initialCall to track what the user originally invoked - Elm.embed(), Elm.fullscreen(), or Elm.worker() - so port-based error messages can focus the user more quickly on the offending invocation.
2. Gives both a more specific error message in the original scenario, as well as a helpful suggestion for how to go about fixing it.

Example

Before:

Uncaught TypeError: Cannot read property 'make' of undefined

After:

Port Error:
The first argument you passed to Elm.fullscreen() was undefined.

This often means no module by that name was actually compiled.

A good thing to check is that your module declaration matches what your first argument to Elm.fullscreen() is trying to reference.

For example, if you are passing `Elm.Foo.Bar`, then the first line of your Bar.elm file should be `module Foo.Bar where`

[evancz]

cc @JoeyEremondi

This might be resolved by the dead code elimination project. Essentially, I think the idea of generating Elm.Foo is flawed for these reasons, so it may make the most sense to switch to giving the module name as a string: Elm.fullscreen('Foo')

This would mean we can do this check in a nice way as well, and I feel like this also fits into the broader plans for generated code better.

[evancz]

cc @JoeyEremondi

This might be resolved by the dead code elimination project. Essentially, I think the idea of generating Elm.Foo is flawed for these reasons, so it may make the most sense to switch to giving the module name as a string: Elm.fullscreen('Foo')

This would mean we can do this check in a nice way as well, and I feel like this also fits into the broader plans for generated code better.

[rtfeldman]

100% agree about using strings instead of nested objects.

Happy to close this in favor of something even nicer ending up in a future release! 😄

[rtfeldman]

100% agree about using strings instead of nested objects.

Happy to close this in favor of something even nicer ending up in a future release! 😄

[JoeyEremondi]

That error looks great! Nothing scares new people off of Elm like seeing a runtime Type error in JS.

Yeah, if we change the interface to be a string instead of object, that should help resolve this.

Changing this might also help another issue that I remembered, pointed out on the mailing list a while ago about Dead Code elimination. Sometimes a piece of Elm code will only be referenced inside of Native code. If references to Elm from Native are all string literals, it will be easy to parse Native JS and search for code that needs to be kept for this purpose.

As a general design principle, I'd ideally prefer that all access of Elm functions and values inside of Native code is done through a specific API. This would let us change the way things are represented internally (for example, optimizing away constructors for "type" definitions with only one constructor, or replacing constructor String equality checks with integer equality checks), without breaking Native modules or peoples' JS wrapper code.

String instead of objects would be a good first-step for this, since it hides how the code is actually generated.

I apologize if this is too off topic for the PR, I can start an elm-dev topic if that would be better.

[JoeyEremondi]

That error looks great! Nothing scares new people off of Elm like seeing a runtime Type error in JS.

Yeah, if we change the interface to be a string instead of object, that should help resolve this.

Changing this might also help another issue that I remembered, pointed out on the mailing list a while ago about Dead Code elimination. Sometimes a piece of Elm code will only be referenced inside of Native code. If references to Elm from Native are all string literals, it will be easy to parse Native JS and search for code that needs to be kept for this purpose.

As a general design principle, I'd ideally prefer that all access of Elm functions and values inside of Native code is done through a specific API. This would let us change the way things are represented internally (for example, optimizing away constructors for "type" definitions with only one constructor, or replacing constructor String equality checks with integer equality checks), without breaking Native modules or peoples' JS wrapper code.

String instead of objects would be a good first-step for this, since it hides how the code is actually generated.

I apologize if this is too off topic for the PR, I can start an elm-dev topic if that would be better.

[evancz]

Not sure what the right resolution is here. Let's revisit based on what's going on with the upcoming release.

[evancz]

Not sure what the right resolution is here. Let's revisit based on what's going on with the upcoming release.