Scoped imports - Githubissues

chocolateboy commented 7 years ago

Crystal doesn't currently provide a way to control the scope of imported bindings e.g.:

cmd = import "command"
cmd.run "echo 'Hello, world!'"

The workaround pollutes the global namespace e.g.:

require "command"

Command.run "echo 'Hello, world!'"

This is fine in many cases, but can lead to surprises, not all of which are caught by the compiler:

Many languages and platforms provide a way to limit the scope of imports (as well as other conveniences such as renaming them to avoid collisions) e.g.:

Groovy
Kotlin
Node.js (CommonJS)
Python
Scala

In addition, Ruby provides refinements as a way to mitigate this problem.

A method for importing exported values has been proposed before, but the discussion quickly devolved into a defence of require and the Ruby culture of monkey patching. As a result, such proposals have (correctly, IMO) been rejected on the grounds that:

we won't change the require system

But there is no need to break/change require. The two concepts are not mutually exclusive e.g. one can do both in Node.js:

const _ = require('lodash') // scoped import
require('shelljs/global')   // global monkey-patch

I'd like to propose a separate/new method which supports scoped imports, and which complements (i.e. does not replace or override) require. If this proposal is going to be rejected, it should be rejected in its own right.

Implementation wise, there could be a new (file-scoped) hook:

main.cr

  cmd = import "command", shell: "zsh"

command.cr

private module Command
  class Runner 
    # ...
  end
end

macro imported(shell = "bash")
  Command::Runner.new(shell: shell)
end

main.cr

cmd = import "command"

command.cr

private class Command
  # ...
end

macro imported(*args)
  Command.new(...)
end

could desugar to something like (this is just a sketch):

main.cr

cmd = CompilationUnit["command"].imported

command.cr

private class Command
  # ...
end

class << CompilationUnit.current
  def self.imported(*args)
    Command.new(...)
  end
end

where CompilationUnit would, I'm assuming, be an internal class.

So if you need to use this transitive type dependency, you need to require the original library which can be painful.

I still don't follow. Can you give an example of how this issue manifests itself in, say, ES6 or CommonJS, which support monkey patching and scoped imports?

using import with multiple namespaces would relax that restriction and you end up with incompatibilities in types between multiple different versions of a library.

Again, this proposal doesn't affect or change namespaces in any way.

faultyserver commented 7 years ago

Sorry for the super-long post. I tried paring it down, but there were a few distinct points I wanted to make that kept it pretty long.

I'm all for implementing something like refinements from Ruby or using from C++, but I don't think a "scoped import" system is necessary or particularly beneficial.

The issue that I see with it is somewhat overarching of the points that RX14 brought up: there would be two distinct paradigms for loading code, and relying on 3rd-party libraries inevitably means that users will have to deal with both in their codebase.

Because there are advantages to both systems, creating a single, idiomatic approach for programmers to follow would be nearly impossible. People's opinions are unlikely to change, and as soon as one library decides to go the other way (use import instead of require, or vice versa), end users have to start remembering which libraries do what and how to include them.

In theory, it's nice to say that require and import should complement each other, but their usage is undoubtably intertwined, and I think most people would (at a glance) see them as interchangeable.

I think it should be the library's responsibility to namespace code, not the user's. Even then, it's possible (and easy) to resolve namespace collisions with a simple re-assignment as I'll show later on, so I don't see any explicit gain in having import.

tl;dr; opinion from the rest of this post: require and import come from two disparate ideologies about global namespaces and including code from other files, and there's nothing one system can do that the other can't. Making both of them available is more confusing than helpful.

Languages like Python, ES6 (I think. at least with the es6 module system), and Java only provide scoped imports. They don't have global imports to a single namespace in the way that require in Ruby or #include in C/C++ provide. For example, something like:

import os

doesn't import os to a global namespace; it is file-private and it's members are still bound to the os namespace. Similarly, using the from ... import syntax:

from math import floor

floor is still a file-private import, even though it has been hoisted from the math namespace.

That's one of the fundamental differences between (in particular) Python and Ruby. Python requires that you explicitly state all dependencies in every file they are used, while Ruby allows you to build up a global namespace that is available everywhere. Each method has its pros and cons, and debating them is pointless, since there will never be consensus (if you haven't gathered already, I'm biased towards the Ruby/C/C++ method).

I think it's important, though, to recognize that Crystal has adopted the require paradigm and the global namespace ideology that goes with it. Tacking on a non-global system seems counterproductive to me, since it doesn't provide anything that can't already be done. The idiomatic way to solve global namespace collisions is (or should be, imo) to have libraries use their own namespaces (modules). Take this ruby example:

# some_library.rb
module SomeLibrary
  class Thing; end
end

# some_other_library.rb
module SomeOtherLibrary
  class Thing; end
end

# no namespace collisions :)
require 'some_library'
require 'some_other_library'
SomeLibrary::Thing
SomeOtherLibrary::Thing

If (somehow) you still have namespace collisions, you can manually rename dependencies quite easily:

require 'some_library'
SomeRenamedLibrary = SomeLibrary
load 'some_library' # using load instead of require simply to reload the same file again

# still no namespace collisions
SomeRenamedLibrary::Thing
SomeLibrary::Thing

This is a contrived example, for sure, but (again, imo) anything that doesn't follow this pattern should be considered non-idiomatic code and refactored to follow it. It's similar to the whole using namespace debacle in C++ (where, fwiw, you still have to #include the code before you can using namespace it)

This hasn't been brought up directly, but I saw a mention of Elixir, so I figure I'll bring it up now. The way that Elixir loads modules is not based on import or require or anything like that. Instead Elixir simply loads all of the .beam files it can find into the runtime, meaning all modules, classes, etc. are always available, and namespacing is even moreso idiomatic there.

import is simply used as an aliasing mechanism, and require is only needed to load macros at compile time.

I didn't know this for a long time, and was confused by the apparent magical loading that Elixir did when I was able to do something like import Ecto.Query without directly giving a specific file to load. I still don't like this approach, because I think it makes reading a codebase more difficult, since Ecto.Query could be defined in one or more files, and I can't know where those files are just by seeing where it gets used.

A similar argument could be made against the Ruby/C/C++ system, but at least there I'll eventually find a file include that shows me where to go. Again, I don't think that's worth debating, it's just been my experience with Elixir.

This is somewhat unrelated, but comparing require and import in Elixir to what they refer to in this discussion would be incorrect, since they don't serve the same purpose.

To clarify what I think RX14 was getting at with the "This proposal namespaces libraries..." point: If a library is written using requires, and then the end user wants to import that library into a contained namespace, what happens with all of those requires from inside the library? Having them contained in that namespace seems extraordinarily complicated, as it essentially changes the semantics of require.

For example, assuming import is made a part of the language and returns a Module as the namespace for the included file:

# some_library.cr
require 'json'

class SomeThing
end

# end_user_code.cr
SomeLibrary = import "some_library"

What namespace does the JSON module get loaded into? Unless require is modified in some way, it would be loaded into the global namespace, in which case the entire purpose of having import (i.e., limiting global namespace pollution) is rendered somewhat moot.

Even if require where to change, how would it be changed to both make included files available in the global namespace and not do that if the code is imported in another file? In terms of code:

SomeLibrary = import "some_library"
# does this work? It would with the way `require` works now
JSON.parse("[]")
# or is it namespaced under SomeLibrary? `require` would need
# to change for this to work
SomeLibrary::JSON.parse("[]")
# or is it not available at all? Again, `require` would have to change
SomeLibrary::JSON # => undefined constant SomeLibrary::JSON

tl;dr again: require and import come from two disparate ideologies about global namespaces and including code from other files, and there's nothing one system can do that the other can't. Making both of them available is more confusing than helpful, and also makes the compiler a lot more complicated when the two can be interspersed in a codebase.

bcardiff commented 7 years ago

@chocolateboy thanks for sharing the refinements talk. It really gives a refreshing look to that topic.

Using refinements in a file level lexical scope is something that could make sense IMO. But there is still "the issue" of the shared global namespace of where that refinement lives. In that sense a require system that is more explicit (and less fun) like ES6 might be more coherent. Finally as highlighted by the refinements presentation, since they are not so widely adopted it might no be clear still if they are a good enough solution.

RX14 commented 7 years ago

@faultyserver you absolutely nailed what I was getting at, thanks so much!

asterite commented 7 years ago

One major concern about the current require system, or at least about the language, are top-level (global) methods and macros. These are not namespaced and so if a popular library choses to use one, then sorry for the other libraries, that name is already taken. And this is not just a theoretical concern: Kemal defines get, post, ws, error, content_for, CONTENT_FOR_BLOCKS, yield_content, render, halt, add_context_storage_type and probably others that I've missed. And I've seen other libraries happily use the top-level namespace for helper functions and macros.

To solve this, maybe we can remove top-level methods and macros altogether. That would force everyone to namespace their declarations. We can't make sure that people will use different module/class names, but chances of collision are now lower.

Now, to achieve something like being able to write:

get "/" do
end

which is nice and convenient, we can maybe have the top-level include make these methods and macros accessible in the current file. So one would have to do something like:

include Kemal::DSL

get "/" do
end

There would be an extra line in these files, which is not that cumbersome, but at least we can sleep without worries at night :-P

Because the standard library already defines some pretty common and useful top-level methods and macros like puts, pp and record, all files could implicitly include Kernel for this, or maybe not.

There's still the issue about reopening existing classes like String and adding methods to them, which can bring some conflicts. Maybe we'd need something similar for that: adding extension methods but scoped to a file (I don't think we need this scoped to a module).

Anyway, these are just thoughts...

asterite commented 7 years ago

In fact, we could still allow methods and macros to appear at the top-level, but these will always be file-private. These are convenient for short scripts or specs.

Hmmm... I might try to implement this and see how it goes, shouldn't be that hard. Then we can really see if it's useful/convenient.

chocolateboy commented 7 years ago

end users have to start remembering which libraries do what and how to include them.

I don't see how this is any different from using any library in any language.

I think it should be the library's responsibility to namespace code

If a library is written using requires, and then the end user wants to import that library into a contained namespace, what happens with all of those requires from inside the library? Having them contained in that namespace seems extraordinarily complicated, as it essentially changes the semantics of require.

This proposal has nothing to do with namespaces and doesn't change the semantics of require in any way. This keeps being brought up as an (abstract) objection, but it has nothing whatsoever to do with the (concrete) implementation outlined here.

Languages like Python, ES6 (I think. at least with the es6 module system), and Java only provide scoped imports.

ES6 supports both, which is why I mentioned it and asked for examples of conflicts between the two usages. Examples include:

import 'source-map-support/register'
import 'shelljs/global'
import 'core-js'

Making both of them available is more confusing than helpful.

Using both is entrenched in JavaScript, in which both monkey patching (i.e. polyfills) and scoped imports (pretty much everything on NPM) are widely used. I've never encountered any "confusion" between these uses. If someone has a concrete example of this, I'm all ears, but at the moment these objections are purely hypothetical and don't reflect real world usage.

makes the compiler a lot more complicated

I'm more than happy if someone else wants to take a crack at implementing this, but at the moment the proposed implementation doesn't add any complexity to the compiler that I'm aware of. It merely sugars something that can (almost) be done already.

chocolateboy commented 7 years ago

In fact, we could still allow methods and macros to appear at the top-level, but these will always be file-private.

Hmmm... I might try to implement this

If you do this, please consider doing it against another ticket. This proposal deliberately avoids namespace-related changes, which are (traditionally) the domain of require.

It would be great if the scope of this discussion could be limited to this proposal and its suggested implementation, rather than going the way of #140, which quickly lost sight of its original topic. IMO, other proposals regarding refinements and namespace changes would be better served by their own separate discussions.

asterite commented 7 years ago

@chocolateboy The proposed implementation doesn't work out of the box. If you return the Command::Runner module which is private, the other file shouldn't be able to access it. The compiler now has to know that this type was brought via an import of some sort.

I'd also like to see some real examples other than just a run method of a Command class. For example, imagine there's a redis library, or Kemal. How would you use these with import?

I think I'm also against having two ways of "importing" code. I understand that JS has two ways, but I believe that's for historical reasons.

chocolateboy commented 7 years ago

The proposed implementation doesn't work out of the box. If you return the Command::Runner module which is private, the other file shouldn't be able to access it. The compiler now has to know that this type was brought via an import of some sort.

This was the first thing I tested, and it appears to work as expected:

test.cr

require "./foo/bar.cr"

test = Exported.imported
puts test
puts test.class
puts test.class.new
puts Command #=> Error

foo/bar.cr

private module Command
  class Runner
    def initialize(@shell = "bash"); end

    def run(command)
      "#{@shell} -c #{command.inspect}"
    end
  end
end

module Exported
  def self.imported
    Command::Runner.new
  end
end

output

#<Command::Runner:0x840f00>
Command::Runner
#<Command::Runner:0x840ee0>

What am I missing?

chocolateboy commented 7 years ago

I'd also like to see some real examples other than just a run method of a Command class.

Sure.

For example, imagine there's a redis library, or Kemal. How would you use these with import?

Doesn't Kemal provide a DSL? If so, why would you use import? Again, this isn't a proposal to replace or augment require.

RX14 commented 7 years ago

@chocolateboy What if I want to use Command in a type restriction? As an instance variable? With your implementation it's impossible because there's no way to reference the type.

asterite commented 7 years ago

It's just that I need a real-world example to consider this (at least me). Because in the previous example we can just have a MyLibrary::Command::Runner and then you can write:

runner = MyLibrary::Command::Runner.new(...)
runner.cmd(...)

Of course you have to be careful to namespace your command runner, but that's what you have to do in every Crystal library. And the amount of typing is more or less the same.

chocolateboy commented 7 years ago

What if I want to use Command in a type restriction?

Use typeof.

As an instance variable?

Have you tried it?

test.cr

require "./foo/bar.cr"

class Test
  def initialize
    @test = Exported.imported
  end

  def test
    puts @test
    puts @test.class
    puts @test.class.new
    # puts Command # Error
  end
end

Test.new.test

output

$ crystal test.cr

#<Command::Runner:0x102aee0>
Command::Runner
#<Command::Runner:0x102aec0>

With your implementation it's impossible because there's no way to reference the type.

It's no more or less possible with this implementation since private modules and classes already exist and are used extensively in Crystal's standard library and specs:

$ rg 'private\s+(module|class|struct|lib|alias|[A-Z]+)' spec/ src/ | wc -l
234

If there are inconsistencies or other issues relating to private modules and classes, by all means raise them in another issue, but, as it currently stands, this proposal doesn't introduce anything relating to them that isn't already implemented.

asterite commented 7 years ago

But here @test = Exported.imported is inferred from the call, which is typed. If we'd have import there would be no way to refer to that type name.

RX14 commented 7 years ago

@chocolateboy In addition to what @asterite said, using typeof() seems like a hack. However - regardless of implementation details - there would have to be a change in the way namespacing works so that certain files could reference certain types and not others.

@asterite I really like the idea of having the top level be always file-private, but how would that work with the stdlib? If you add a special modifier to make top-level things be public then we'll probably end up back where we started.

chocolateboy commented 7 years ago

But here @test = Exported.imported is inferred from the call, which is typed. If we'd have import there would be no way to refer to that type name.

This is just a reified example of the implementation sketched above i.e.:

test = import "./foo/bar.cr"

would desugar to (something like):

test = CompilationUnitForFooBar.imported

Are you saying this isn't possible?

RX14 commented 7 years ago

@chocolateboy There are always times in which the type inference for class vars isn't possible, so you have to specify the type explicitly. It's often that you want to do this for explicitness. This proposal makes you use typeof() for that situation which is an ugly hack to a very common situation. To resolve this you'd have to change namspacing/requires/the compiler. And i'm not sure that the typeof hack would work in all situations with an unmodified compiler.

chocolateboy commented 7 years ago

using typeof() seems like a hack.

This proposal makes you use typeof() for that situation which is an ugly hack to a very common situation.

Feedback on, and criticism of, private modules/classes belong elsewhere (e.g. on #3280), rather than here, since this proposal doesn't introduce or impact that functionality in any way.

I really like the idea of having the top level be always file-private, but how would that work with the stdlib?

By all means close this issue if it is to be superseded by a different implementation, but please take the discussion of that different implementation to a different issue. We already have an example of a derailed version of this discussion in #140. Let's please try to keep at least one version of it focused on the actual proposal.

asterite commented 7 years ago

@chocolateboy Maybe it's possible. But foo = CompilationUnitForFooBar.imported doesn't exist yet, though it can be simulated, but the file has to be processed in a different way (I guess you don't want declarations in that file to pollute the global namespace).

I'm still looking for real examples where one library that you currently use with require would benefit with using import instead.

I'll soon write a proposal about changing the top-level include which will also improve the situation regarding types.

chocolateboy commented 7 years ago

(I guess you don't want declarations in that file to pollute the global namespace).

No, that's a mischaracterisation spread in this discussion, but which has never been part of this proposal. I'm not proposing any changes to namespaces/visibility or require. It would be entirely up to the author whether Command (or whatever) is public or private. Returning a value from an imported file is orthogonal to that.

The only thing new in this proposal is the imported hook and the (internal) CompilationUnitForFooBar object/class it's called on.

asterite commented 7 years ago

I mean, if you have:

# file: "one.cr"
class One
end

macro imported
  One.new
end

# file: "two.cr"
one = import "./one"

# This compiles fine, because I didn't say One was private
One.new

Is that expected behaviour?

I also find it a bit strange that a file just imports methods, where one would generally like to use types, and possibly refer to types inside that file. How would you handle that case?

chocolateboy commented 7 years ago

Is that expected behaviour?

Yes. Just the same as JavaScript:

test.js

const fooBar = require('./foobar.js')

foobar.js

// FooBar.baz requires the Array.flatMap polyfill
Array.prototype.flatMap = function () { } // global side-effect

const fooBar = new FooBar(...) // local value
module.exports = fooBar

Though I doubt this would be the typical use case.

faultyserver commented 7 years ago

I think the main reason that namespacing is being discussed here is the second line in the original post:

The workaround pollutes the global namespace e.g.:

But there is no need to break/change require. The two concepts are not mutually exclusive e.g. one can do both in Node.js:

Node's require is always scoped. It's similar to Python in that there is no global, shared context*. You can do a bare require without an assignment because there is an implicit or explicit module defined that the file's contents are inside of, and that module just gets assigned to the file scope. Assigning to a variable just acts as an alias for that module in the file scope.

Because Crystal does have that global, shared context, something about the require system would have to change to implicitly or explicitly modularize every file. That could be on the user to do, or it could be done implicitly by require. The latter seems impractical, because you already can do the former, which begs the question of why import is even necessary when you can already solve the problem with good, idiomatic code.

* there is some shared state with window and things in the standard library, which I find annoying to say the least.

chocolateboy commented 7 years ago

I also find it a bit strange that a file just imports methods, where one would generally like to use types, and possibly refer to types inside that file. How would you handle that case?

Again, there appears to be some confusion between this proposal and require. The import method proposed here is just a runtime method call, which invokes a method which can return any value, including types (i.e. classes):

test.js

require "./library.cr"

library = Exported.imported

puts library[:Foo]
puts library[:Bar].new.value

library.cr

private class Foo
  def value
    "foo"
  end
end

private class Bar
  def value
    "bar"
  end
end

module Exported
  def self.imported
    { Foo: Foo, Bar: Bar }
  end
end

output

Foo
bar

chocolateboy commented 7 years ago

Node's require is always scoped. It's similar to Python in that there is no global, shared context.

As mentioned earlier, modifying the global scope and reopening global classes via require or import is not only possible in JavaScript, but common (thanks to polyfills).

RX14 commented 7 years ago

@chocolateboy

The import method proposed here is just a runtime method call, which invokes a method which can return any value, including types (i.e. classes)

The problem is that this is impossible to do in crystal. You cannot return a type from a function, only a metaclass, and using typeof() everywhere is a huge smell.

faultyserver commented 7 years ago

The second part of that paragraph is what's important, though:

You can do a bare require without an assignment because there is an implicit or explicit module defined that the file's contents are inside of, and that module just gets assigned to the file scope.

AFAIK, Crystal doesn't implicitly define a module (or even CompilationUnit, as it were) for each file, everything just gets tacked into the global namespace (which is why that's a relevant thing to discuss).

If Crystal were to start defining modules like ES6, it would undoubtedly change (at least the implementation of) require, because it would have to unwrap that module and copy over objects into the global namespace, or else it's semantics would change and require would be no different than import in the first place.

FWIW, as long as require doesn't change, I don't care what happens with this either way. I'm not going to use it, so it doesn't matter to me.

chocolateboy commented 7 years ago

using typeof() everywhere is a huge smell

If that's your use case, use require. The purpose of this proposal is to provide a way for a file/compilation unit to return a value, which require doesn't do. It is not, and has never been, a way to replace require.

chocolateboy commented 7 years ago

AFAIK, Crystal doesn't implicitly define a module (or even CompilationUnit, as it were) for each file

It doesn't currently, though it clearly can be done. (The compiler does use a CompilationUnit class internally. I haven't investigated whether it can or should be reused for this purpose.)

asoffa commented 7 years ago

Correct me if I'm wrong, but this proposal seems to be to add something roughly like

# in file use_foo.cr
Foo = import "path/to/foo"  # or `import "path/to/foo" as Foo`, `use Foo = "path/to/foo"`, etc.

# becomes syntactic sugar for something like
private module Foo
  extend self
  require_only_into_this_module_and_dont_ignore_even_if_previously_required "path/to/foo"
end

Users who require/import use_foo.cr will then never know (unless they examine the code) about Foo or what goes on inside Foo.

To those who have objected to this: How, then, could any confusion possibly arise?

chocolateboy commented 7 years ago

The current proposal is more like this:

main.cr

test = import "./foo/bar.cr"

foo/bar.cr

private class Whatever
  # ...
end

macro imported(baz)
  Whatever.new(baz)
end

- which would desugar to something like:

main.cr

test = CompilationUnit.import_foo_bar

foo/bar.cr

private class Whatever
  # ...
end

internal class CompilationUnit
  def self.import_foo_bar(baz = nil)
    Whatever.new(baz)
  end
end

import is just a method call and it wouldn't automatically inject or export (or hide) types any more than any other method call. This proposal is limited to providing a way to return a value when a file is required.

asoffa commented 7 years ago

@chocolateboy Interesting...would this require (pardon the pun) library authors to define a private class + imported macro as you have above in order to use this feature, or would the compiler do this automatically upon encountering an import?

chocolateboy commented 7 years ago

The compiler wouldn't hide anything automatically. The user would define an imported hook if they want the file to return a value. They are not required to hide or expose any modules or classes in the file. That's orthogonal to this feature.

asoffa commented 7 years ago

Very interesting! Mind = blown :-)

faultyserver commented 7 years ago

A question about that import hook:

If I write a library and don't implement an imported macro, but a user of the library wants to import rather than require it, how does that work?

My understanding of your proposal is that the end user wouldn't be able to import the library, then. Would the end user have to make a new file (or edit the library) to define the imported hook and use that?

Maybe I'm missing something...

faultyserver commented 7 years ago

If the solution is "library authors should always implement macro imported", then I don't think we've gained anything, since we could just as easily say "library authors should always namespace their code in a module" and get the same result.

chocolateboy commented 7 years ago

If I write a library and don't implement an imported macro, but a user of the library wants to import rather than require it, how does that work?

Why would you want to import a value from a library which doesn't export a value? That makes no sense. It's literally the same as expecting to get a value from a method which doesn't exist.

As I've stated in almost every reply in this thread (!), this proposal doesn't replace require or change its semantics. It doesn't require library authors to change their modules, library consumers to change their code, or Crystal hackers to change their habits.

we could just as easily say "library authors should always namespace their code in a module" and get the same result.

It's not the same result since it requires pollution of the global namespace.

This proposal provides a way for a file to return a value when required. There is currently no way to do that without polluting the global namespace e.g. with the Exported module shown above. This proposal provides a way to do that, and nothing else.

faultyserver commented 7 years ago

Why would you want to import a value from a library which doesn't export a value?

That's kind of the point I'm making, though. Say I'm a dumb library author and make a library that puts hundreds of entries into the global namespace, and for some reason a user wants/has to use it, but with import so those entries are instead encapsulated.

Is that not the exact use case for import? I honestly don't see any other reason that it would be used.

this proposal doesn't replace require or change its semantics.

Of course it doesn't. But, import would act as a complement to require so we have to think about how the two interact when both are used in a codebase.

It's not the same result since it requires pollution of the global namespace.

So does doing test = import "test.cr" at the top-level.

If I import something that requires something else, what happens?

The same thing that happens if you require something that requires something else. This proposal doesn't change the behaviour of require. There are no changes to namespaces or namespacing.

So if I write a library and load all of my dependencies with require, and you want to use it with import to scope it to a variable, only the file that directly gets included would be scoped, then all of the dependencies that my library includes would still be in the global namespace? E.G., if I require "json" in my library, will that still be exposed globally even when you import it?

If the response is "why would you import something like that?", see my first point. If the answer is that JSON would not be exposed globally, then the semantics of require would have to change to either a) make it file-private, or b) somehow recursively track imports/requires to see how each are being used and magically determine how to load the code from there.

chocolateboy commented 7 years ago

but with import so those entries are instead encapsulated

import doesn't automagically make a library encapsulated. As I've repeatedly stated, hiding or exposing modules is orthogonal to this proposal.

So does doing test = import "test.cr" at the top-level.

Not for me:

test.cr

require "./foo/bar.cr"

test = Exported.imported

test

$ crystal eval 'require "./test.cr"; puts test'
Error in line 1: undefined local variable or method 'test'

if I require "json" in my library, will that still be exposed globally even when you import it

Yes, as has already been stated.

I honestly don't see any other reason that it would be used.

There are plenty of use cases for returning a value from a compilation unit without requiring pollution of the global namespace, including several issues outlined in the OP, but I don't see the point in going over them again when you've made it clear that:

as long as require doesn't change, I don't care what happens with this either way. I'm not going to use it, so it doesn't matter to me.

faultyserver commented 7 years ago

You're right. I don't particularly care, and I don't think it's worth me debating anything about this anymore since it's clear that we have two very different views about what this change would mean, and I don't have any authority to make a decision.

I would honestly recommend drafting a PR with this change to prove that it doesn't affect require or impose a burden on library authors, etc. Those seem to be the two main concerns that people have, and I doubt just talking about it will convince anyone.

If it works, then there can be a proper debate about whether this is good for the language. So far, it's just been speculation about whether it's even possible without affecting other things. (Bonus points that if it works and gets accepted, it can be immediately merged).

asoffa commented 7 years ago

More evidence from other languages that import and require can play nicely together:

Python has both import and execfile (Python 2) / exec (Python 3)
Julia has both import/using and include
Nim has both import and include

Granted that in these languages the import way rather than the require way is the primary means of using code from other files, I have yet to see anyone complain about having both available

asoffa commented 7 years ago

Just to make sure I'm reading this correctly (there has been a lot of back-and-forth discussion above), this proposal does not attempt to solve the issue of polluting global namespace for cases of using someone else's poorly namespaced library - is that correct? i.e. When I write my own library, I have no choice but to pollute my global namespace if that's what is done in my dependencies, and with the import feature I can control global namespace pollution only for items within my own library?

Let's say that a particular library author decides not to use this import feature at all and that I need to require many files which recursively require many files that pollute the global namespace to a level of high annoyance. I then cannot isolate these requires into a file where I define an imported hook that returns only a module or class instance (i.e. without leaking all of the requires in the file)?

If this is the case, then this doesn't really provide a solution to the issue raised in #4368 - I would still be "held hostage" in the sense of being forced to include globally for both myself and my users whatever the authors of my dependencies decide to make global or are forced to make global. So e.g. any upstream dependency using lower-level libraries would still end up leaking all of its low-level dependencies...unless all authors of all of the lower-level dependencies have sufficient foresight to use the import hook feature to the satisfaction of all downstream users of their libraries. This issue is mediated at least partially with #4442. Is this limitation really necessary?

(If the issue I mention here is beyond the scope of the issues this proposal is trying to address, I can move this discussion to #4368.)

konovod commented 7 years ago

Well, at least you will be able to write wrapper that will require the library and export single interface with all needed functions. Remaining code will import this wrapper and remains not polluted. I think it's not so great as full-featured module system, but still better than nothing.

asoffa commented 7 years ago

@konovod If I can indeed write such a wrapper file to contain the requires, then this proposal provides a near-perfect solution. However, based on the above discussion, it is sounding like the import would still leak the requires...

konovod commented 7 years ago

Yeah, sorry, i wasn't paying enough attention. If importing a file will bring everything that this file require to the global namespace, the feature doesn't look very useful.

crystal-lang / crystal

Scoped imports #4439

main.cr

command.cr

See Also

140

3280

4368

main.cr

command.cr

main.cr

command.cr

test.cr

foo/bar.cr

output

test.cr

output

test.js

foobar.js

test.js

library.cr

output

main.cr

foo/bar.cr

main.cr

foo/bar.cr

test.cr

test