-
-
Notifications
You must be signed in to change notification settings - Fork 24
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
Revamping the import system #220
Comments
I don't understand what this means. You mean every module in my own project or the entire system? Won't this in both cases lead to potential namespace collisions a lot? |
A few downsides for this:
I'm sure other uses-cases like this exist, even though they will be quite rare.
This can make sense only if the previous feature is supported. But would also require qualified usages for basic functions, which I'm not sure is an experience you'd want 🤔 Btw, with these changes, it's likely you won't have any imports anymore . A reminder that if you try to document the first declaration and you have no imports nor a module documentation, then that documentation will act like the module documentation, not the declaration 's documentation.
Just to be clear, this is a a command to generate a new module with some imports already there, right? It's not a customizable prelude? I think you'll find Basics imported on 99% of all projects too, so might as well include it directly too. Do we need Platform.Cmd imported by default, probably not no. So I think you could alter the prelude to have less things, but I wouldn't remove everything from it. (I don't think a prelude will be great either btw)
Sounds reasonable. Maybe you could even provide from which source-directory you could import the module.
Could you re-expose the constructors as well? If so, I think this sounds good. If not, then I'm not sure what the value is over adding a type alias. You would have two ways of doing the same thing.
This one I think is not a great proposal. The separation of exposing is what bugs me the most though. Often, if I need to see if an API exposes a function that does X, it's much faster to scan for the names in the exposing list than the docs. With this proposal, the exposing are far in between and it becomes a hassle to find what I'm looking for. The problem you're introducing trying to solve here is about not having missing @docs notation. If I'm right, then I think there are better solutions. We could have a "@docs for others" without the duplicate exposing, though I think that would actually worden the resulting docs. We could have a compiler error when you're missing @docs. We have it for packages, but not for applications and non-exposed modules. That's what elm-review-documentation is for, until there is a compiler check. |
Very nice proposal 👌 There is a couple of things I’d like to comment on: Maybe the one that doesn’t seem very necessary is the multiple exposing one. You could argue you can interleave comments in the exposing list too, with a similar result. Something like this
Although maybe it is not so aesthetically pleasing as the multiple exposing one. —- Would you consider changing —- What is the problem of keeping a small focused default import? It really helps readability for the basic types of the language. There are many things in Basics that could be moved to better named modules so that accessing them qualified would read nice, like all of the math functions, which could go under Math (Math.sin, etc). Then the basics default import could be stripped down to the bare types and constructors every app will use, and you don’t need the generate cli shenanigans and all files look a bit cleaner. |
It means that
These are good points. A long term goal of Gren is to rewrite the compiler in Gren. That would include making the most of the compiler a package which can be relied on for things like formatting, linting, analysis etc. Would that help in this case? The compiler has the same basic need of figuring out which packages have changed in order to do a incremental re-compile.
Right. However, you would only get a collision if you were to use a conflicting name in practice, if that helps? If you define an alias, which happens to collide with another module, you won't be having a conflict as the compiler will already know which module you're referring too.
Not if
Right. And again, a good point, but considering the operators as sugar proposal (#219), you might want to whitelist certain imports anyway?
You will find that Gren's prelude (specifically
In Gren you can have local dependencies. So you make a copy of the previous version to a new folder, change the
Yes
Not sure I understand your criticism. My proposal essentially makes a
I think this is a valid argument. Would cause a ton of breakage though 😅 I'll consider it!
Gren already has a With the operators as syntax sugar proposal, we wouldn't want the typical operator names to be a default import (or you wouldn't be able to override them), and with both that proposal and parametric modules, most functions will move to seperate modules anyway (compare will move to If you don't have to specify imports, then there's no need to have a So, in my mind at least, the future set of default imports would be
And the operator functions you'd want to be explicit anyway in order to be able to override them. |
I'm not sure what elm-review does, but for a compiler (at least for one I worked on), for figuring out the dependencies of a module I needed to do the parsing of the module to get the imports out of it. In that case, given the unique syntax of Gren, doing the parse of the module would give you the modules it depends on as well from the code, so in both cases you have to do parsing.
My bad!
This reminds me that we do use a lot the import statements for bringing in the main type of the imported module for using in type signatures. With this proposal, given less imports will be used, it means likely that people will move to use qualified type names for type signatures, which in my opinion can be ugly. Or end up having to write most imports anyways just to bring a type to scope. Something to ponder about.
This module system looks a lot like OCaml and F#. I know in OCaml they usually name the main type of a module as
I don't think F# has the same convention, they do name their types with readable names, but I'm not sure how they deal with modules, because they use |
Hmm. Could we have a local |
In the JS community the |
Since no goal is stated regarding the removal of the doSomething =
internalFn
|> Task.andThen otherInternal
|> Task.attempt ResultingMsg What module is doSomething =
internalFn
|> Task.andThen otherInternal
|> Task.map2 (\now other -> { now, other } Time.now
|> Task.attempt ResultingMsg They make the incorrect assumption that we're using the core A nice error message will likely help them, something like
but by this point we're already wasting the developer's time. |
@laurentpayot bear in mind, @wolfadex Got it. I've been thinking of requiring that aliases are unique, which would make the example you posted above not compile. I believe Roc has the same decision. Would that work for you? @boxed I'm leaning towards not allowing merging of module names. While convinient, it does add some confusion, see @wolfadex 's comment above. |
|
@laurentpayot I see what you're saying. But in JS, the thing that immedietly follows the |
@robinheghan I’m a bit confused. Do you mean we could have something like this in Gren? import MyModule from "skinney/package" exposing (foo) |
@laurentpayot yes. But |
@robinheghan Oh I see now. That makes sense 👍 |
This is exactly the stuff that is being mentioned, when discussing having multiple aliases. Right now it is possible to alias more than one module to the same name in the imports, which means that you can fake extend a module by providing functions from another module to the same name. It’s on Robin to decide if this is a behavior he wants to keep with this import system overhaul, or if it is something that he wants to disallow. A probably incomplete summary: The pros:
The cons:
I personally love being able to write clean code and use this pattern for merging Extra modules with the original module, makes me feel in can extend built-ins and the code reads a lot better for it. I’d prefer it if I could do this and the people that don’t like it can do their own thing and enforce lints to disallow duplicate aliases rather than removing this nice little feature. |
Just edited the proposal.
|
I agree with joakin here, this is actually something I really like about Gren/Elm. It improves readability a lot. It's something I used heavily in my first Gren package. I had to put stuff in different modules to avoid recursive imports, but I then alias them all to the same module in the code that uses the package for readability. (see Markdown example: https://packages.gren-lang.org/package/icidasset/shikensu-gren/version/4.0.0/overview) I agree that it makes it a bit harder to tell where a function resides, but I think most people that have experience with writing Gren/Elm code look at the imports first before assuming where the function comes from. I think the good outweighs the bad in this case. That said, maybe there's a way to write my code better so I can avoid the aliasing? Would re-expose fix this? Love all the other sub-proposals 👏 |
I really don't like this. One thing I've grown to appreciate about Elm modules is their self-documenting design – you can look at the top of a module and see exactly:
This proposal takes away that second benefit from Gren, for the very minor convenience of not having to explicitly specify your imports. This creates problems for anyone coming into your codebase (or your future self coming back to an old codebase) and trying to understand how all of the modules are related to one another – you'd have to scroll through each module and look for the names of all the modules being implicitly imported. This doesn't seem like a worthwhile tradeoff to me. If the goal is to encourage the use of qualified references, I think your other sub-proposals are already pointed in that direction – I'm not seeing how this particular sub-proposal is necessary to achieve that goal. If the goal is to allow the programmer to write their initial code more quickly, importing available modules without having to manually key in the related |
Hi Gabriel, thanks for chiming in!
This is interesting. Do people actually read the list of imports before reading looking up definitions in a module? I get it for modules containing < 100 lines of code, because then it's actually possible to keep a list of imports in your head, sort of. But in the projects I've worked on, with hundreds of modules and some modules containing 1000-2000 lines of code with 20-30 imports, the import statements are pure noise to me. I've ended up always writing imports of the form This proposal would make the way I write import statements to be the easy way, which means you're more likely to see qualified references and know which constant is being referred to regardless of import statements.
When is it relevant to know which modules are being imported into a module? 🤔 If this is a true need, this can be implemented as an operation in the compiler:
This is an option. Although it would only work on source code, not in the repl. |
I would like to chime in here that in Swift this is how it works. I find this jarring, but I'm also a hobby level Swift coder. It's easy to get name conflicts for one, which is annoying. Swift also has quite bad compile times, and I wonder if this has something to do with that. I don't know...
I think that's ok personally. The repl is for interactive use and should have some different tradeoffs if you can cut down on typing. A repl could also do this:
ie, print the imports (maybe in another color) that are implicitly run. |
I could be wrong, but in my memory Swift has one namespace per package. So everything in a single package is in the same scope, more or less. Usually you attach methods to I also belive qualified imports isn't common in Swift (unqualified being the norm), which is another way in which my proposal differs. |
Ah, yea ok, I misremembered what the original proposal was 😅 Just having access to the module names seems much more sane to me, even though I personally don't like |
Each feature here could be presented as an independant proposal, but since they all center around the same system, it makes sense to present them as one.
Every module is available without an explicit import
Just as it says on the tin. If you don't need to specify an alias or import things unqualified, then you don't need an import statement at all. This should greatly reduce the need for imports, and encourage using qualified references.
No more default imports
The recent operators as syntax sugar proposal would benefit from being able to choose which function is called when an operator is used. This doesn't work too well with default imports.
Making every module available without explicit import statements helps somewhat, but there are certain values that would be nice to have imported by default, like
Just
andNothing
.To help with this, the CLI will get a
generate
command which can generate a new module with a specific set of imports included in the template. This way, the default imports are explicit, and can be removed or changed according to the whims of the programmer.A
from
keywordWe don't really have a good way of dealing with collisions in module names. This proposal simply adds a
from
keyword that can be used to select which package a module should be imported from, as a way to select a winner between colliding modules.Move documentation comment above module declaration
This will be more inline with how documentation comments work in all other parts of the language, and also avoid ambuguity when the import section is empty (which is more likely if the above proposals make it into the language).
Other than moving the location of the documentation comment, no other changes are made to the module's documentation comment.
Dissallow modules sharing a name or alias
Gren currently allows two modules to share a name (common example is aliasing Array.Extra as Array, to merge those two namespaces). This makes it hard to tell where a function really resides, especially when the import section isn't available (diffs or tutorials). It can also make it harder for external tools to know where a value comes from.
Suggestion removing this ability.
The text was updated successfully, but these errors were encountered: