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

Remove support for «exposing (..)» from import statements #76

Open
robinheghan opened this issue Jun 28, 2022 · 7 comments · May be fixed by #84
Open

Remove support for «exposing (..)» from import statements #76

robinheghan opened this issue Jun 28, 2022 · 7 comments · May be fixed by #84
Labels
accepted The issue has been reviewed and accepted for implementation good first issue Good for newcomers help wanted Extra attention is needed language proposal proposal of a major feature to the language

Comments

@robinheghan
Copy link
Member

No description provided.

@robinheghan robinheghan added language proposal proposal of a major feature to the language accepted The issue has been reviewed and accepted for implementation labels Jun 28, 2022
@robinheghan robinheghan added this to the 0.2.0 milestone Jun 28, 2022
@robinheghan robinheghan self-assigned this Jun 28, 2022
@lue-bird
Copy link
Contributor

why not include module exposing (..) in this proposal?

@robinheghan
Copy link
Member Author

Good question.

I believe this was tested in elm-format (where it automaticly converted (..) to a list of all constants), but this was later reverted. I don't remember the reasoning for that, but @avh4 might?

Personally, I think module exposing (..) is fine in two cases:

  1. Early prototyping. I rarely figure out what an API should be before it's close to finished. Not having to deal with that at an early stage is a productivity boost for me.
  2. In modules where everything actually is meant to be exposed, having a syntax for exposing everything makes it very clear to the reader what is going on. This is especially nice in very large, possibly generated, modules. Example: https://github.com/robinheghan/elm-phone-numbers/blob/1.0.0/src/PhoneNumber/Countries.elm

@lue-bird
Copy link
Contributor

lue-bird commented Jun 28, 2022

Someone was writing multiple independent tests at the module level and was just surprised when elm-format had automatically converted module exposing (..) to expose the current members only but not the new tests that were added afterwards. avh4/elm-format#578

  1. That's not an issue when the syntax is forbidden in the first place
  2. This was fixed in elm-format by only adding explicit exposes to module ...\n lines module lines with no exposing clause will have the clause automatically generated avh4/elm-format#625 (great solution IMO)

To your arguments:

  1. I've personally never done this, and I don't see the minimal inconvenience of click + expose as a hindrance worth adding syntax for (others do). Maybe that's just different styles of designing APIs
  2. In normal modules, one basically can't ever guarantee that everything at the module level should be exposed. For generation modules, explicit exposing is handled by the generation tool anyway. Again, not worth adding syntax for.

The strongest argument I can see is your first one, which, as wolfadex pointed out, could be solved by treating exposing everything as a warning and an error in production/optimized mode just like Debug calls.
This could be done through separate syntax like the old module exposing (..) or module exposing todo or for example just an @exposing-todo tag in the module doc comment which reminds gren format to expose new module level members.

@robinheghan
Copy link
Member Author

  1. True, this could be a debug-only feature.
  2. I work in projects where Elm modules are used both for translations and configuration. In both these cases, all members should be exposed, and it's not generated. In both cases exposing (..) is easier to read than a long list of exposed members.

@lue-bird
Copy link
Contributor

lue-bird commented Jun 29, 2022

To 2., of course I'd need deeper insight:

What's the benefit of

module Translation exposing (..)

picture =
    { de = "Bild", en = "picture", ... }

settings =
    { de = "Einstellungen", en = "settings", ... }

vs for example

module Translation exposing (for)

for =
    { picture =
        { de = "Bild", en = "picture", ... }
    , settings =
        { de = "Einstellungen", en = "settings", ... }
    }

I guess something to do with types etc. which can't be replicated with let for example?

Anyway, your examples seem like valid cases where exposing (..) can be helpful. Do they offset the otherwise negative possible consequences? If yes, gren's official documentation should make the use-cases and pitfalls clear.

@robinheghan
Copy link
Member Author

In our particular case, we compile one app per language. So our translation files look like:

module Translation exposing (..)

picture = "Bild"

settings = "Einstellungen"

... a lot more ...

We then use a tool like linked-folder to symlink the correct translations before build.

The benefit of not wrapping it all in a record is improved static analysis, dead-code elimination and a flatter namespace.

Not saying this is the best way to do it, but it works very well for us.

@robinheghan
Copy link
Member Author

If yes, gren's official documentation should make the use-cases and pitfalls clear.

Agreed.

@robinheghan robinheghan added the good first issue Good for newcomers label Jul 22, 2022
evuez added a commit to evuez/gren-compiler that referenced this issue Aug 7, 2022
evuez added a commit to evuez/gren-compiler that referenced this issue Aug 7, 2022
@evuez evuez linked a pull request Aug 7, 2022 that will close this issue
@robinheghan robinheghan removed their assignment Aug 20, 2022
@robinheghan robinheghan added the help wanted Extra attention is needed label Sep 2, 2022
@robinheghan robinheghan removed this from the 0.3.0 - Testing & Debugging milestone Jun 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted The issue has been reviewed and accepted for implementation good first issue Good for newcomers help wanted Extra attention is needed language proposal proposal of a major feature to the language
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants