elm-tooling / elm-language-server

Language server implementation for Elm
https://www.npmjs.com/package/@elm-tooling/elm-language-server
MIT License
419 stars 65 forks source link

Depedency resolving needs to load from elm-stuff first #453

Closed razzeee closed 3 years ago

razzeee commented 3 years ago

Depedency resolving needs to load from elm-stuff first and then fall back to the global packages.

jmbockhorst commented 3 years ago

Not sure if reading elm-stuff is the problem you are having. There is a problem in node-elm-review where an application elm.json has elm-review as a dependency, and elm-review has elm-explorations/test as a dependency. However, elm-explorations/test isn't listed as an indirect dep like it should be (As far as I'm aware). The compiler is able to resolve this without the indirect dep, but we aren't. Maybe this is what you are seeing as well?

Fyi @jfmengels

razzeee commented 3 years ago

Might be, but my test case is not with elm-review and only happens after deleting the global package cache

jmbockhorst commented 3 years ago

Oh okay. Well I looked at the compiler code, and it uses d.dat to store the last "compile time", and if the elm.json modification time equals the last compile time, it doesn't need to do anything, everything is cached. In our case, if we wanted to support this, we would have to read all the cache files in elm-stuff to get the data (all dependency functions, types, etc), and it would be a lot of work to get everything working without source files (We wouldn't have any dependency source code to parse, just cache data).

Personally I don't think this is a use case we need to worry about, right now anyways.

jfmengels commented 3 years ago

Just FYI, the reason why there is a missing indirect dependency for that project (and a few others in elm-review-land), is because even though it should be there, elm-test crashes when it finds the same package in multiple places. That has been fixed in the upcoming elm-test release (revision 5), so in that case, that should be an issue for very long anymore.

razzeee commented 3 years ago

Agreed, will close this. This should be enough documentation.