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

Some modules are not clickable in function signatures on package.elm-lang.org #35

Closed
daphee opened this issue Nov 10, 2017 · 4 comments
Closed

Comments

@daphee
Copy link
Contributor

daphee commented Nov 10, 2017

Have a look at the documentation for Point3d.distanceFromAxis

In the function signature Point3d is clickable and leads to the Point3d module and type. Axis3d on the other hand isn't. It seems quite a few other modules have this issue as well.

@ianmackenzie
Copy link
Owner

Yeah, this is a known issue but tricky to resolve because Elm doesn't allow true cyclic dependencies between modules. However, the Point3d module has to refer to the Axis3d type for the type signature of Point3d.rotateAround, and the Axis3d module has to refer to the Point3d type since that's what its origin point is!

The current solution is to have all actual types defined in the OpenSolid.Geometry.Internal module, and then each module declares an alias to one of those internal types:

module OpenSolid.Point3d

import OpenSolid.Geometry.Internal as Internal

type alias Point3d =
    Internal.Point3d
module OpenSolid.Axis3d

import OpenSolid.Geometry.Internal as Internal

type alias Axis3d =
    Internal.Axis3d

In most cases, modules can directly import the modules they depend on and just use the type aliases declared in those modules, for example

module OpenSolid.Axis3d

import OpenSolid.Point3d exposing (Point3d)

This means that the Point3d type in the Axis3d function signatures is actually the alias defined in the Point3d module, so that turns into a clickable link to the Point3d module. However, to break the cyclic dependency the Point3d module just directly imports the internal Axis3d type instead of importing the alias from the Axis3d module:

module OpenSolid.Point3d

import OpenSolid.Geometry.Internal expoing (Axis3d)

Since this is actually the same type as the exposed alias in the Axis3d module, the type checker is happy, but since the Axis3d type referred to in the Point3d module is not actually publicly exposed, there's nothing to link to in the documentation. In general, links from higher-level to lower-level modules should work but not the other way around.

The only way I can think of to solve this would be to have a public OpenSolid.Geometry.Types module which just has a bunch of aliases to the internal types; then each type could just have a doc comment with a link to the relevant module. But then it gets messy - do you also have aliases in each module which are aliases for the aliases in the Types module? What's the canonical way to refer to any given type from some other module?

@ianmackenzie
Copy link
Owner

Unless you can think of some other clever solution =) But I've discussed this issue with a bunch of other experienced Elm developers and nobody has come up with a perfect answer. I may indeed end up switching to an OpenSolid.Geometry.Types module, but I think that would imply that user code would have to have imports like

import OpenSolid.Geometry.Types exposing (Point3d, Axis3d)
import OpenSolid.Point3d as Point3d
import OpenSolid.Axis3d as Axis3d

and I prefer the more standard approach that the current structure allows:

import OpenSolid.Point3d as Point3d exposing (Point3d)
import OpenSolid.Axis3d as Axis3d exposing (Axis3d)

So yes, I think I'll close this for now - feel free to reopen if you have more ideas, and thanks for raising the issue! Now there's something I can link to the next time this comes up =)

@daphee
Copy link
Contributor Author

daphee commented Nov 10, 2017

Yeah, the current approach definitely looks nicer.

If I come up with a good solution you'll get PR directly 😉

Sorry for removing my previous comment. It felt obvious and I had hoped you hadn't read/started responding yet. For reference, it said:

Thanks a lot for the thorough explanation! So this is pretty much unfixable, right?

@ianmackenzie
Copy link
Owner

It occurs to me that splitting opensolid/geometry into opensolid/primitives and opensolid/geometry (as proposed in opensolid/meta#1) could help resolve this, since the 'primitive' types (points, vectors, directions, bounding boxes, intervals, axes, planes, sketch planes and frames) have a lot of cyclic interdependencies but the higher-level geometry types (line segments, triangles, arcs, splines, spheres, polygons etc.) do not. It could be that opensolid/primitives could have actual exposed types that you would import as

import OpenSolid.Primitives exposing (Point3d, Axis3d)
import OpenSolid.Point3d as Point3d

etc. but opensolid/geometry would have proper opaque types defined directly in their own modules (no Internal module) that you could import like normal:

import OpenSolid.Sphere3d as Sphere3d exposing (Sphere3d)

I kind of like that...seems OK for the primitives to be a bit of a special case if it means everything else can be nice and clean.

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

No branches or pull requests

2 participants