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

Consider Namespacing library dependencies #40

Closed
ChristophP opened this issue Jan 1, 2020 · 5 comments
Closed

Consider Namespacing library dependencies #40

ChristophP opened this issue Jan 1, 2020 · 5 comments

Comments

@ChristophP
Copy link

Problem
elm-3d-scene makes use of bunch of other libraries. A couple of them are not namespaced for people (like me) who haven't used those before it is difficult to see where the imports come from.

import Angle -- ianmackenzie/elm-units
import Camera3d  -- ianmackenzie/camera-3d
import Color -- tesk9/palette
import Color.Transparent -- tesk9/palette
import Direction3d -- tesk9/palette
import Length -- ianmackenzie/elm-units
import Pixels exposing (Pixels, pixels) -- ianmackenzie/elm-units
import Point3d -- ianmackenzie/elm-geometry
import Scene3d exposing (noDirectLighting, noEnvironmentalLighting) -- ianmackenzie/elm-3d-scene
import Scene3d.Chromaticity as Chromaticity -- ianmackenzie/elm-3d-scene
import Scene3d.Drawable as Drawable -- ianmackenzie/elm-3d-scene
import Scene3d.Exposure as Exposure -- ianmackenzie/elm-3d-scene
import Scene3d.Mesh as Mesh -- ianmackenzie/elm-3d-scene
import Triangle3d -- ianmackenzie/elm-geometry
import Viewpoint3d -- ianmackenzie/camera-3d

Suggested Solution
Namespace the modules in the other libraries like Unit.Length instead of Lenght for elm-units. Other affected libraries are elm-geometry, elm-3d-camera and possibly more.

Other Implications
Namespacing would also help dodge the fact that the compiler currently cannot diambiguate modules with the same name. See here

Other
If there is a decision to implement what this issue suggests, it may be better to track on the respective repos of the libraries. But until them I felt it would give a better insight over the issue here.

@ianmackenzie
Copy link
Owner

elm-geometry (well, really its precursor opensolid/geometry) actually used to have prefixed module names, but I moved away from that a while ago in favor of plain Point3d, Triangle3d etc. A bunch of the relevant discussion is in ianmackenzie/elm-geometry#22, but the short version is that non-prefixed module names seemed to be the community standard and that there are probably better ways to avoid the naming collision issue than package authors adding prefixes everywhere. (Personally, I think it would be nice if modules could optionally be imported with a syntax like import Point3d from ianmackenzie/elm-geometry as Pt3d, but there are probably lots of other potential solutions). I'm also not aware of any current module naming conflicts with any of my packages, so I don't think an urgent fix is needed.

I agree that it can be confusing figuring out which packages different imports come from (you're not the first and probably not the last person to bring this up), but I'm not particularly eager to reopen the prefixing/namespacing discussion right now. What probably would make sense, though, would be to annotate the examples much like you've done above, so that people reading the examples can immediately see what's happening. Thoughts?

@ChristophP
Copy link
Author

ChristophP commented Jan 2, 2020

Hi Ian, I see it actually used to be that way that the modules were namespaced. Given the history I understand you don't wanna reexamine everything again.

Annotating examples: I think that would solve the confusion for beginners. The only problem with it is that once you run elm-format over it, it will pull all the comments up over the block of imports. I described it in this issue here.

Naming collisions: Totally agree that having something like import X from author/user would be great to disambiguate, but the last issues I've seen haven't really moved since 2017. There may not be current clashes YOUR packages, but I could still see some problems ahead, because the name clashing also affects indirect dependencies. This means that for example no one using elm-3d-scene will be able to use any other Color module than tesk9/pallete. Would be a shame if people had headaches because their dependencies don't line up with elm-3d-scene's. I although I realize now, that even namespacing everything will not solve this problem. :-/

@ianmackenzie
Copy link
Owner

Good point about the elm-format issue - that's a bit annoying, but nothing that can't be worked around if necessary.

Are you sure that name clashing also affects indirect dependencies? I don't think that's true...I mean, you certainly get compile errors if you try to import a module in a package you only indirectly depend on (it's happened to me several times that I get a compile error for import Json.Decode because I haven't explicitly installed elm/json, even though it's in my indirect dependencies). To me that implies that something like import Color would work as long as I directly depended on only one of the color packages, even if I had the other as an indirect dependency somehow, since it certainly seems that the compiler hides modules from indirect dependencies during the import resolution process.

I also think namespacing at this point is a short-term fix that might avoid some issues in the near term while at a potential cost to the Elm language/ecosystem in the long term. You're right that there hasn't been much recent movement on things like elm/compiler#1625, but until Elm gets close to 1.0 I think things like module import syntax should still be considered to be in flux. And in order to figure out the best solution, Evan et. al. need case studies of where there are issues with the current design...such as instances where naming conflicts actually come up! So any naming conflicts avoided right now by adding a namespace would mean fewer case studies to consider, making it harder to figure out the right long-term solution. On the flip side, if not adding a namespace now means naming conflicts do come up, then there's a bit of pain to work around in the near term, but it provides a good data point for figuring out what the long term module import solution should be.

@ChristophP
Copy link
Author

ChristophP commented Jan 3, 2020

Are you sure that name clashing also affects indirect dependencies?

Now that you put it that way I am not 100% sure. You may be right about that. If it doesn't affect dependencies that is a very good thing. I may have confused that case with versions because the compiler can not resolve multiple version of the same library as far as I know.

I agree with the rest of your statement, that namespacing now will prevent gathering data points for a proper fix in the language.

On the flip side, if not adding a namespace now means naming conflicts do come up, then there's a bit of pain to work around in the near term

Not sure if what that bit of pain would be, because I think once you have those issues there's not really anything you can do to my knowledge besides namespace then to avoid it.

Having heard your reasoning, I think it's fine to leave module names as they are for now. Since we done quite some "Considering" I think we can close the issue. :-)

@ianmackenzie
Copy link
Owner

You're definitely right about versions - indirect dependencies can certainly cause version constraint conflicts. (And of course if you use ianmackenzie/elm-3d-scene then you'll probably have to directly depend on tesk9/palette yourself anyways, which means you will get module import conflicts if you also depend directly on avh4/elm-color.)

I think it would be possible to work around things in some cases by vendoring certain dependencies into your own repository and placing them under a prefix there; for example you could depend on elm-3d-scene and then copy the avh4/elm-color source files into your own repository under a prefix or something. You could also do something like publish a forked version of elm-3d-scene that used avh4/elm-color or used a vendored/prefixed version of tesk9/palette or something. Perhaps describing that as "a bit" of pain is pushing it, but I think there are workarounds in at least some cases.

Anyways, yes, I think we're all pretty clear on what the issues/tradeoffs are so I think I'll close this for now. If at some point it becomes clear that import resolution is not going to change, then I'm happy to reopen the discussion - although ideally as part of a broader conversation within the Elm community, so that there can be some consistency between packages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants