Galooshi / import-js

A tool to simplify importing JS modules
MIT License
525 stars 55 forks source link

Use absolute paths #339

Open trotzig opened 8 years ago

trotzig commented 8 years ago

I think it would be safer for us to always use /absolute/paths. Right now, we're inconsistent about paths. Most of the times, they're ./local to the current working directory. Sometimes they're also local/to the current working directory. And occasionally they're /absolute. I think we can minimize confusion by using absolute paths.

rhettlivingston commented 8 years ago

I've looked at the source for a lot of projects lately that do javascript source processing, and absolute paths is definitely the norm for referencing the source files and as keys for pools of source files.

Not just absolute though,,, also make them real - no symlinks. Symlinks lead to duplicate keys for the same file.

lencioni commented 8 years ago

I agree that it is important that paths are consistent. I'm not entirely convinced about the benefits of /absolute/paths. I think it might be safer, but it will add a little complexity to configuration, since consumers will need to take in the path to the project when writing configuration functions. I think it would be good to have some examples of how this affects configuration before implementing absolute paths.

lencioni commented 8 years ago

And with symlinks, I'm also not entirely sure about that because if you are writing some configuration based on the location of the symlink, you need to match on the path of the symlink, not the real path.

trotzig commented 8 years ago

good point about configuration @lencioni. I have a vague memory of a jest (or was it eslint?) issue not long ago related to absolute/relative paths. Can't recall exactly what it was though.

rhettlivingston commented 8 years ago

Agreed and struggling somewhat on both points. Though, I think they could be handled by converting everything to absolute when reading the configuration. That would actually help the match, right?

The whole path thing has been a struggle. I see why EcmaScript pretty much punted on the issue and left it to the packaging and build systems to worry about.

I've been thinking that we need several paths for every real module. My thoughts haven't gelled as to where they are stored. They could possibly all be on a location object of some sort that might have some helper methods.

  1. a true absolute path that serves to be able to reliably read the file and as a unique, internal key.
  2. a path showing the file relative to its project root, possibly using symlinks. Note that for a file inside a package, I think this can be different from the root of the project we're working with. Hence the reason I didn't just fully dismiss that "lookupPath" still hanging around in JsModule.
  3. the path of the file's project root (the root of the node module in some cases, the app in others, a meteor package in others).

Part of what is causing my expansion in thought has to do with the fact that I'm already processing Meteor package files contained in ~/.meteor/packages, i.e. outside of the project directory. It is possible that each of those could get their own EsModules pool and help reduce this problem.

This has caused me to wrestle a bit with whether the processing of a package should create a new EsModules pool. If it does, then (3) above could possibly be kept with the pool. Then the following thoughts could work well.

Really speculating here. An alternative that might work (and be a unique key) is to reference every file the way it would be referenced from the application using "absolute" paths. One "path" in which

  1. a local module would always start with a '/' (or './'?) and be "relative" to the project root
  2. a file from an npm module would always start with the npm package name 'material-ui/checkbox/index.js' and be translated by a method to '/node_modules/material-ui/checkbox/index.js' for a scanner.
  3. a meteor module would always start with 'meteor/' and require a helper from the environment to find the real file. i.e. the environment could map 'meteor/accounts-base/account-client.js' to '/home/rhett/.meteor/packages/accounts-base/1.2.9-beta.9/os/account-client.js'.

The method that can turn any of these things to a filename could be in the EsModule abstract class and subclasses could override the implementation as needed. Hooks would need to be provided to give the environments a shot at augmenting it too.

thebarty commented 7 years ago

Hi guys, just starting out using your package (with meteor).

I'd love to just have absolute path and totally agree. Having absolute path is the safe way to go. If the path ever changes, you simply search&replace the whole project, which you would NOT be able to do with relative path (which makes it compatible with the user developers in the team NOT using atom and your plugin)

ANYWAY... right now it might be a good fit, to have "namedExports" NOT trying to make the path relative, p.e.

namedExports : {
  // this already WORKS fine
  'meteor/check': [
    'check',
    'Match',
  ],
  // this already WORKS fine
  'meteor/practicalmeteor:sinon': [
    'sinon',
    'spies',
  ],
  // this does NOT work and tries to my the path relatvie,
  //  p.e. ``../../../../..//imports/modules/whatever/server/fixtures/common.js``
  '/imports/modules/whatever/server/fixtures/common.js': [
    'CONSTANT_1',
    'CONSTANT_2',
    'CONSTANT_3',
  ],
},

Right now trying to import CONSTANT would give me a ../../../../..//imports/modules/whatever/server/fixtures/common.js which makes this thing unusable for me.

I hope that there will be a fix soon, as this would really help save some time and brainpower! 👍

trotzig commented 7 years ago

@thebarty By absolute paths in the example above, are you referring to paths relative to the project root? This issue is mainly about using absolute file paths internally (starting from the file system root), but it does touch upon project relative paths too.

thebarty commented 7 years ago

@trotzig : Yeah I am. I'd love to have an option to have this package ONLY generate absolute path...

actually my project root is like this

/  // git root
/docs/*  // docs
/src/*  // meteor source
/src/imports/*   // modules that I often import and I'd love to have absolute path for
/.importjs.js  // config

Do you see any change in getting this to work?

trotzig commented 7 years ago

I think you should be able to get what you want using a combination of useRelativePaths: false and a custom moduleNameFormatter adding a leading slash to the path.