Refactor/move file utilities with Meteor 1.3 Imports

waldgeist · June 21, 2016, 4:12pm

I’m curious why the rest of the node community hasn’t run into this

Well, that’s part of that NodeJS mess we wanted to get rid of by using Meteor. SCNR.

serkandurusoy · June 21, 2016, 4:40pm

So, can anybody think of a situation where relative paths are essential?

I think we should move away from packages anyway, (since we are embracing imports) so that’s no longer an issue for the long run, but here are my pet peeves:

since meteor accepts imports from the imports folder only, and that’s not a top level project folder, all imports have to begin with /imports/ (or imports/ if it is to be considered a special imports root, per the build system (meteor) configurations)
at least webstorm gets confused with absolute paths. when I tried it, it completely lost the ability to resolve module exports properly. this leads me to think that absolute imports
absolute paths may mean different things depending on the environment (browser/node), you may even end up trying to read from the file system root!
it is also different from the perspective of the build system. for example, typescript and es6 treat paths differently (so I heard)

Also, I found https://nodejs.org/api/modules.html#modules_all_together an interesting read!

Anyway, if we actually do find out that using absolute paths is working fine, I’d still use a mix where files enclosed in a module use relative paths between each other and modules import from each other using absolute paths. Combining this with index.js files that rexport from the module’s private files, we’ll get a proper structure which we can later than have the ability distribute as public/private npm packages as well.

rlivingston · June 21, 2016, 6:06pm

I second that. I’ve been watching this thread wondering why everyone has found it so difficult because I haven’t felt that way. One of the answers is that I used absolute paths for everything not very very closely related to the module I’m in. So, for example, if I’m in /imports/api/groups/methods.js and I need to import something from the users api, I use the absolute path, ‘/imports/api/users/users’. But, if I’m importing something else within the groups api, I use the relative path, ‘./groups’. This has made it easy to move whole groups of associated files around and just do a global find/replace on the path.

Of course, I started using modular languages 30 years ago and had no IDE support for at least the first 10 years - never really got used to the refactoring tools.

My perspective on the wisdom of introducing imports goes like this.

We’re moving to NPM as a packaging system, a move that will open doors both to the Meteor community to employ and deliver more packages to more users and to the NPM community to benefit from more of Meteor’s goodness.
NPM currently uses “require”.
We were fixing to have to start using “require” all over the place and encounter all of these problems anyway.
But ecmascript has already decided the winner in the war amongst systems for modularization and chosen “import”.
Everyone, including NPM, will eventually change to “import”.
Ben has given us a wonderful gift in that instead of moving to “require” and then having to shift again to “import”, we can jump straight to “import”. I applaud the timing of this! It had to happen and better now than later!
Furthermore, by implementing it as an NPM module, “reify”, he is bringing “import” to the whole NPM community, now. Note also that he was careful in his latest “reify” code to extend the ecmascript just a little, in a way that NPM absolutely requires, so that it can FULLY replace the “require” syntax. i.e. the latest versions allow import to be used in conditional and nested scopes. No more "require"s required.
All of this and other changes coming in 1.4 and 1.5 puts Meteor in the position of entering the NPM community on the bleeding edge instead of the trailing edge.
When at the bleeding edge, development tool pains always occur. We simply need to be as proactive in pushing the tools as MDG has been in pushing Meteor into a position where it can join the larger community and pull it instead of dragging it.

I am right this moment spending a portion of my time trying to get modifications introduced into at least one of the parsers that ESLint uses, either espree and/or babel-eslint, so that conditional and nested imports don’t cause parsing errors that kill all analysis of the modules containing them. i.e. I identified an upcoming problem that will effect not only our community but anyone in the NPM community using ESLint and Ben’s “reify” package to replace “require” with “import” and I’m working to fix that problem before people see it. I’m not an MDG employee, nor is anyone paying me for any effort I’m making today.

So, I applaud everyone on this thread exploring what WE can do to move development tools into position and hope that this discussion will result in solutions that can be put in the guide for everyone’s benefit instead of requests for someone else to do something. If you can’t do it yourself, you can still help greatly by searching for things that are open source and close to what is needed (as I’ve seen some are doing) or brainstorming positive ideas that might give inspiration to those on here who may have the skills and time necessary.

seba · June 21, 2016, 6:18pm

Well, actually I use it a bit more nuanced than I first said.
I do use relative paths for files I know will always reside in the same directory. Like index files and my scss and template imports of my angular components. And a disadvantage as @serkandurusoy also points out is that editors don’t get absolute urls. But that last problem might disappear if we completely move isopackages to NPM packages. But we’re probably still quite a bit away from that.

rlivingston · June 21, 2016, 6:25pm

You are absolutely right in all of your assertions that the contents of the “module-name” in the imports syntax were not specified by the committee. Every build tool has the freedom to interpret it differently. I believe though that Meteor and NPM both will be using it as relative to the app or package directory. We just need to get our tools fixed to support our build system’s interpretation of “module-name” which I don’t believe is unusual.

Note that ESLint was at first marking absolute paths as unfound and a fix for that system is already available in the form of the NPM package “eslint-import-resolver-meteor”. I have submitted a PR to the Guide to add this.

seba · June 21, 2016, 6:30pm

Btw, does somebody know if the name of the index file is configurable?
I’d like to name it _index.js (so prefixed with underscore), that way it would always show up at the top of my directory in webstorm (and most other IDE’s).

joshig · June 21, 2016, 6:51pm

Why do all the Mantra apps not use a /imports/ folder? Have they done something different that I’m missing? @arunoda

eg: https://github.com/mantrajs/meteor-mantra-kickstarter

serkandurusoy · June 21, 2016, 6:54pm

@seba

I’d like to name it _index.js (so prefixed with underscore), that way it would always show up at the top

I do the exact opposite, prefix everything with _ so that index.js is always at the very bottom

@joshig

Why do all the Mantra apps not use a /imports/ folder?

It may be related to the .babelrc file

{
  "presets": ["es2015", "stage-2", "react"],
  "plugins": ["react-require", "babel-root-slash-import"]
}

which uses babel-root-slash-import but that’s still confusing because .babelrc was not supported with meteor back then. Does it perhaps use a different build system like webpack? What gives?

joshig · June 21, 2016, 7:23pm

That’s where I’m confused too, they don’t use any other build system. There is a webpack example, but I’m not using that and most projects I’ve seen don’t use that unless they really want HMR.

Either way, I’m not using /imports/ and don’t seem to be running into any problems at all. Based off that kickstarter though. It’s a topic I’ve always been curious of. I did find this: https://github.com/kadirahq/mantra/issues/172 it seems Meteor is still loading itself, but following the import paths provided if available?

rlivingston · June 21, 2016, 7:28pm

Exactly. And they are now having trouble because of this.

serkandurusoy · June 21, 2016, 7:37pm

@joshig, apart from the issue @rlivingston dug out for us (thank you buddy) a simple

git clone https://github.com/mantrajs/meteor-mantra-kickstarter.git
cd  meteor-mantra-kickstarter
meteor npm install && meteor run

has a host of issues with dependencies etc, so I could not even get to a point to properly test if the imports work!

But, I did try out importing a file (template.html) located outside the imports folder a few days ago only to see the server crash with a module not found error and that was when I found out about the imports folder not being optional.

yelongren · June 22, 2016, 1:57am

As someone working on a small minimalist app, I feel strange having to refactor by adding lines of code with no immediate benefits, in the form of import statements. I remember when the first meteor tutorials prided themselves in having very few lines of code.

When imports become the default, it would be nice to add an ‘eager’ folder so we don’t lose that functionality, or an ‘eager’ prototyping package just like autopublish/insecure.

arunoda · June 23, 2016, 5:59am

@joshig

I fixed babel-root-slash-import issues and now it should work properly.
See: 1.3.3 imports breaking

joshig · June 23, 2016, 5:25pm

Thanks @arunoda! I’ll give it a try very soon! You’re awesome

serkandurusoy · June 24, 2016, 7:32am

For those of you who use webstorm, absolute imports do work with a trick!

/imports/something/somethingelse/somefile.js
/imports/something/somethingelse/index.js

import { /* does not work */ } from '/imports/something/somethingelse' 
import { works } from '/imports/something/somethingelse/index' 
import { works } from '/imports/something/somethingelse/index.js' 
import { works } from '/imports/something/somethingelse/somefile' 
import { works } from '/imports/something/somethingelse/somefile.js'

This means, the presence of index.js is not detected, so the workaround is to append /index to the paths. I think this is a fairly acceptable workaround where one can:

harness full power of webstorm
use absolute paths for inter-module dependencies
use reative paths for module-private dependencies

Edit: Thanks to @sam for https://youtrack.jetbrains.com/issue/WEB-21450

Edit2: One should still be careful with the refactor > move action since not all paths seem to get updated correctly.

Edit3: Well, there’s been a confusion of sam’s, and it should have been @ffxsam whom I owe the thanks to not that I am not thankful to Sam Hatoum for other stuff

batist · June 24, 2016, 11:55am

In webstorm you can set a resource root, so your

import thing from '/import/api/things/things.js;

will be nicely resolved. (It also autocompletes paths & filenames)

serkandurusoy · June 24, 2016, 2:36pm

@batist the problem is not the resource root, since it is already set by default for the project.

The problem is, index.js file. That cannot be resolved with absolute paths.

So for a file path: /import/api/things/index.js

import thing from '/import/api/things/index.js';

and

import thing from '/import/api/things/index';

and

import thing from '../../../api/things';

do resolve while

import thing from '/import/api/thing';

does not. It is a webstorm bug and is being tracked on that link from my previous post.

This bug indeed has adverse affects on file move type refactoring operations.

sam · June 24, 2016, 2:48pm

Ha, that’s funny. That’s a different Sam H. Perhaps it’s @ffxsam? Could also be an imposter

rlivingston · June 26, 2016, 2:40am

It’s not a complete refactoring tool, but the atom-import-js package adds the following keybindings

Cmd+Shift+j	Import word	Import the module for the variable under the cursor.
Cmd+Shift+i	Fix imports	Import any missing modules and remove any modules that are not used.
Cmd+Shift+k	Go to word	Go to the module of the variable under the cursor.

Note Fix imports!!!

I installed it, commented out one of my imports, went to my first no-undef error, hit Cmd+Shift+j, and voila, it found the right module and put an import for it at the top of my file.

Obviously, it won’t all be that easy. The import I commented out was for one of my local modules, not something defined in a package. But, there are a ton of configuration options that I am now exploring to see if I can teach this more about my project.

I also noted that they have an “environments” option which can currently only be set to “node”. Perhaps they need help implementing a “meteor” environment?

There is a batch rewriting capability mentioned at the bottom of their README!!!

awatson1978 · June 26, 2016, 2:50pm

Oh, snap! This is looking very promising! Can’t seem to get Cmd-Shift+I working in Atom, but Cmd-Shift-j and Cmd+Shift+k seems to be working just fine.

From what I can figure, .imports.json should look something like this:

{
  "appliesTo": "**",
  "useRelativePaths": false,
  "namedExports": {
    "/imports/ui/components/PageContainer": [
      "PageContainer"
    ]
  }
}

The only problem is that the we need a leading / at the beginning of the import statement. Does this have anything to do with babel-root-slash-import? Also, having to specify a list of namedExports to get the brackets around them is going to be troublesome (but not the end of the world).

Otherwise, seems to be working well.