meteor / meteor-feature-requests

A tracker for Meteor issues that are requests for new functionality, not bugs.
Other
89 stars 3 forks source link

No more imports directory #135

Closed marioblas closed 6 years ago

marioblas commented 7 years ago

Migrated from: meteor/meteor#8341

elie222 commented 7 years ago

I hope this one doesn't happen. We have a legacy project that still has a lot of code that relies on globals. All new code uses imports, but the fact that we can have both right now and old code doesn't break is amazing and it would be a big loss if we couldn't update to future versions of Meteor due to this change happening.

I don't fully understand the benefit of this change either. If you don't want globals don't use them. The current system allows for both which should make everyone happy.

mitar commented 7 years ago

I think this should be done, but not be the default. It could be a flag in package.json which turns it on for the app. Also, new apps created with meteor create could have this flag turned on.

crapthings commented 6 years ago

no

mitar commented 6 years ago

But it would be also important then to have a directory for client and server with "always import" semantics.

theodorDiaconu commented 6 years ago

Sometimes eager loading is useful, I find myself often creating a new meteor project, going directly to /server, and creating some methods, some api endpoints, and it just works nicely. But for a serious project I only have 2 proxy files in client/main.js and server/main.js that do nothing but import from /imports. It feels a bit stupid. I feel like Meteor is the perfect hybrid between fast prototyping and a serious enterprise app. We all started with it because of its simplicity, let's not take this away.

Besides this, I also think that eager-loading for client-side will be something useful later in the future, especially if a pattern will emerge that defines routing at component level (Just a hunch)

@route("/list")
class Component {}

If we want a complex solution we can go with something like:

package.json or meteor settings, or a special .meteor/config.js file (I can't say which is best))

"meteor-load": {
     "eager": {
        "client": ["./client"],
        "server": ["./server"], 
        "both": ["./lib"]
    },
    "entry": {
        "client": "./imports/startup/client/index.js",
        "server": "./imports/startup/server/index.js"
    }
}

But if we do this, what impact would it have on reloading ? Would we still benefit from a server-only restart if I modify something from server ?

Another elegant way I foresee is:

meteor --entry-client="" --entry-server=""
andyrichardson commented 6 years ago

I think only importing files which are specified would be a huge improvement.

It would also allow for building inside of the project directory and preventing this error:

WARNING: The output directory is under your source tree.
Your generated files may get interpreted as source code!
Consider building into a different directory instead
meteor build ../output
benjamn commented 6 years ago

Given the valid concerns that have been raised in this thread (thanks!), I just want to (re)assure you all that this feature will be implemented in a way that adds flexibility in terms of which directories/modules are eagerly loaded, rather than just eliminating the imports mechanism, and of course the changes will be opt-in—an important principle for us (i.e., backwards compatibility whenever possible).

benjamn commented 6 years ago

This is happening!

gaurav- commented 6 years ago

An ulterior motive for this feature is to enable Meteor apps to have directory layouts that developers who are not familiar with Meteor can immediately understand. The special meaning of the imports directory and the surprising eagerness of modules outside of imports have always required some explanation, so this change should reduce that surprise.

@benjamn that's a very noble ulterior motive :) Personally. I'd be looking forward to simplifying the configuration of my meteor+webpack build after this upgrade.

How would it affect the "absolute" import paths though? i.e. would the equivalent of '/imports/module/path' be '/module/path' or module/path? I hope it's the latter; that would match the convention followed by most non-meteor apps using es modules.

benjamn commented 6 years ago

It’s relative to the root directory of the application, so I think you would want imports/module/path or ./imports/module/path, without an initial / character.

If we made absolute imports (with an initial /) work (and I think we probably should, just for completeness), it would still be relative to the application root, not the file system root.

Does that answer your question?

gaurav- commented 6 years ago

@benjamn, my question was actually entirely focused on the first paragraph of your answer. You're right that I want imports/module/path to work. So if I understood correctly, that's how it would be in 1.6.2 after opting-in to this feature, right?

Just to give you some more context of my setup: the imports directory of my app is a symlink to a common folder outside the meteor app, that is also used by webpack to bundle the client-side. This shared code has import paths like module/path. Thanks to babel-plugin-module-resolver and some webpack configuration, importing such "base paths" (without initial / or .) works the same way in both meteor and webpack builds.

As a side note, I've always found the absolute imports (with initial /) to be misleading because it's not the real root of the filesystem. I'm agnostic about supporting it with this new feature though, as long as it doesn't come at the cost of the behavior described above 🤞

benjamn commented 6 years ago

If that setup works with Meteor now, I’m pretty sure we can keep it working with this feature, though I would encourage you to try the latest beta 1.6.2-beta.12 to be sure (there’s still plenty of time to fix it if it doesn’t work!).

gaurav- commented 6 years ago

I tested a minimal app created with:

meteor create --release 1.6.2-beta.12 --minimal test-meteor-1.6.2

Works: import '/module/path'; Doesn't work: import 'module/path' (throws Error: Cannot find module 'module/path')

Can't test my whole app for now, but hope to do so some time next week.

If it's open for consideration, I'd suggest supporting the base path imports so that:

  1. Meteor can remove yet another point of surprise, since import paths with initial / seem to be uncommon in other popular conventions like create-react-app.
  2. It would be (relatively) easier to share the same code between meteor and other bundlers. In my own case, I'd be able to reduce (maybe even eliminate) the configuration code specifically for that purpose.
benjamn commented 6 years ago

@gaurav- You need to do import ./module/path, otherwise it looks like you're importing the path module from a module package installed in node_modules. Changing this behavior is not open for consideration, since this is how the entire Node/CommonJS/ECMAScript ecosystem works.

merlinstardust commented 6 years ago

Which release is the mainModule in and when will it be the official release? I discovered via History.md that 1.7 is available, but there's nothing on the blog, and it was not the set release for my new project yesterday.

benjamn commented 6 years ago

@merlinpatt Meteor 1.7 (and 1.7.0.1) has it!

Given that new Meteor apps no longer need an imports/ directory (because they use meteor.mainModule), and existing Meteor apps can opt into this functionality, I think we've satisfied the spirit of this feature request!

sakulstra commented 6 years ago

follow up question here :) is cordova treated as modern or legacy or is there a separate entry point - which one is used if none is specified for cordova?

awatson1978 commented 5 years ago

What great work. Thank you, thank you, thank you! Finally refactored the /imports directory to /client/app and then back to /app. So great to have back the ability to structure the directory layout how we want!!! Thank you!!!!