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

Add pseudo-encapsulated require #4461

Closed
asoffa opened this issue May 25, 2017 · 5 comments
Closed

Add pseudo-encapsulated require #4461

asoffa opened this issue May 25, 2017 · 5 comments

Comments

@asoffa
Copy link

asoffa commented May 25, 2017

The conventional way to introduce a new library in Crystal is to use a global module, and Crystal does not currently provide an automated way of dealing with naming collisions.

I propose a system that effectively eliminates the need to introduce a new global when creating a new library. This is accomplished by a new keyword: package (or maybe shard or something similar). A package is a global module which is dynamically named at compile time and which is meant to be used in a private context as the following example will show:

# in library.cr:

package Library  # gets transformed into something like `module _UniqueIdentifierForLibraryDotCr_Library`
  # ...
end

Now let's see how to put Library to use in a project:

# in use_library.cr:

require "path/to/library”

# gets expanded to the current
require “path/to/library”
private alias Library = _UniqueIdentifierForLibraryDotCr_Library


# and if I want a different name:

require "path/to/library”, {“Library” => “LibraryWithMyOwnName”}

# gets expanded to
require “path/to/library”
private alias LibraryWithMyOwnName = _UniqueIdentifierForLibraryDotCr_Library


# also want to make the library available to your entire package? no problem:

package MyPackage
  private alias Lib = LibraryWithMyOwnName
end

This proposal plays well with the require system + monkey patching and provides a solution to all issues raised in #140, #4368, #4439 without breaking require in any way as far as I can tell.

The specifics are of course up for discussion.

Thoughts?

@konovod
Copy link
Contributor

konovod commented May 25, 2017

I'm not sure it is solving #4368 completely - if e.g. shard A use shards B,C,D, and user program use only shard A, then require 'a' will bring all public (not covered under package) interface of B,C,D to global namespace leading to namespace collisions and other problems.
Of course that is IMHO still better then nothing and better than "scoped, but not scoped" imports.

Hmm, if all shards follow the same rule and never require directly, then all modules will be private so nothing from B,C,D gets to user namespace. So it's partially solving #4368

@asoffa
Copy link
Author

asoffa commented May 25, 2017

@konovod Yes, the expectation would be for all libraries to use this package/shard system if adopted. There would be no reason not to AFAICT (with a few exceptions: #4439 (comment)). Though you are correct that this proposal doesn't prevent people from writing bad code :-)

@ysbaddaden
Copy link
Contributor

Please don't just throw some technical solutions without explaining the rationale. This subject is controversial and has been rejected a few times. Be more explicit about what you're trying to solve; refer to actual problems that actually happened, not personal anecdotes, or the argument that some other languages do this (so what? I don't use it) or worse: the possiblity that something may happen someday.

The actual require is simple, and has no complexity. Conventions solve potential naming conflicts. Why would I even start to think about using your solution?

@asoffa
Copy link
Author

asoffa commented May 25, 2017

Why would I even start to think about using your solution?

Because it doesn't modify existing behavior at all while solving all of the issues (and many were mentioned) detailed throughout #140, #4368, #4439, and probably others I have missed. Are you saying you don't see any issue mentioned in those tickets that is an actual issue? If so, I will dig up others, because there are others.

My question is: if you have the option of having naming conflicts, why would you choose the option of being forced to allow them over eliminating them? This proposal provides a tool (that you can choose to use or not to use) to eliminate this maybe infrequent, but significant problem. (Is there a reason a conflict is to be desired?) While we're at it, let's get rid of private so that even more naming conflicts can arise. (Yes, private adds another keyword to the language, but eliminates an entire class of problems, just like this proposal.)

The complexity added to require in this proposal is minimal AFAICT (though I am not an expert, so I may be missing something here): in almost all cases there would be only one package used per file. If there is any unforeseen complexity, an identifier telling require to ignore packages (or a new keyword altogether such as use, require_pkg, etc.) could be used in cases where it's not wanted.

Every rejection to similar proposals that I have seen so far has boiled down to not wanting to modify/break the require system, e.g. #140 (comment). This is a proposal that finally does just that: keep require as-is (with the small, invisible, maybe optional addition illustrated above). And modifying/breaking the require system has been the only objection so far

@asterite
Copy link
Member

I'm closing this. If one day a different way to require code appears, it will come from the core team. Please don't send any more proposals related to this. Thank you.

@crystal-lang crystal-lang locked and limited conversation to collaborators May 25, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants