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

external resolution #104

Closed
ondras opened this Issue Sep 3, 2015 · 23 comments

Comments

Projects
None yet
8 participants
@ondras
Copy link

ondras commented Sep 3, 2015

@Rich-Harris, please, how is the external resolving procedure supposed to work? Say my ES6 module uses

import x from "y.js";

as far as I know, "y.js" shall be resolved with respect to the current resolution root, which would be -- in the case of a webpage -- the page's BaseURL.

But rollup now resolves this module ID by traversing the tree upwards, scanning for node_modules with a jsnext:main-equipped package.json. Is this a work in progress, or a final solution?

What if the current node-ish approach only worked for extension-less module IDs (import x from "stuff"), leaving "y.js" available for paths relative to the import root?

@Victorystick

This comment has been minimized.

Copy link
Member

Victorystick commented Sep 3, 2015

Rollup currently considers all non-relative paths to be references to external modules. You're already familiar with the default method of resolving these module ids.

If you're using Rollup through the JavaScript API, you can override the resolveExternal function to get the behaviour you want.

var rollup = require( 'rollup' );
var path = require( 'path' );

rollup.rollup({
    entry: 'entry.js',
    // Resolve externals against the base directory...
    resolveExternal: function ( id ) {
        return path.resolve( __dirname, id );
    }
}).then( function ( bundle ) {
    bundle.write( {
        dest: 'bundle.js'
    } );
}

What if the current node-ish approach only worked for extension-less module IDs (import x from "stuff"), leaving "y.js" available for paths relative to the import root?

That could be a possibility.

@ondras

This comment has been minimized.

Copy link
Author

ondras commented Sep 3, 2015

Rollup currently considers all non-relative paths to be references to external modules. You're already familiar with the default method of resolving these module ids.

If you're using Rollup through the JavaScript API, you can override the resolveExternal function to get the behaviour you want.

I am happy with the ability to specify a custom resolveExternal, although I am only using a CLI version at the moment.

I am, however, puzzled by the decision to treat all no-leading-dot module names as external (absolute paths being apparently an exception). Is there anything inherently external in this use case?

/* this file located at ui/views/main/pane.js */

import * as rpc from "util/net/rpc.js";

/* ...more accessible than "../../../util/net/rpc.js" */
@Victorystick

This comment has been minimized.

Copy link
Member

Victorystick commented Sep 3, 2015

I think the reason is compatibility with the Node ecosystem, and other ES6 transpilers. There's a reason the #1 issue is concerned resolvers; there is no obvious best solution.

Since #95 is on its way, I'm not sure how easy it'd be to satisfy your use case per default.

How about a leading /, to signify the resolution root?

import * as rpc from "/util/net/rpc.js";
@ondras

This comment has been minimized.

Copy link
Author

ondras commented Sep 3, 2015

How about a leading /, to signify the resolution root?

Sounds fair for my particular use case. But I am pretty certain that some others would prefer to have support for absolute (with respect to local filesystem) paths as well.

Back in the old CommonJS days (even before Node), the module ID resolution procedure worked roughly as "Names that do not start with a dot are searched in a list of search roots (module libraries)". Perhaps rollup can maintain this approach, creating a list of search roots by starting with pwd, continuing with node_modules and so on?

@ondras

This comment has been minimized.

Copy link
Author

ondras commented Sep 3, 2015

Also, do we have some summary/comparison on how other ES6 module processing tools handle these edge cases (missing extension, non-slash non-dot name, ...) ? Would be useful.

@Rich-Harris

This comment has been minimized.

Copy link
Contributor

Rich-Harris commented Sep 3, 2015

I hear you about utils/... being much nicer (and refactor-friendly) than ../../../utils/.... Unfortunately it turns out to be a real problem when you have a situation like this:

/* /path/to/lib/main.js */
import x from 'x';
import myOtherLib from 'myOtherLib';

/* /path/to/lib/x.js */
export default 'it finds this just fine'

/* /path/to/lib/node_modules/myOtherLib/src/index.js */
import y from 'y';
export default 'it also finds this file ok'

/* /path/to/lib/node_modules/myOtherLib/src/y.js */
export default 'but it can\'t find this, because it resolves y against /path/to/lib'

There's no reliable way to know which directory in myOtherLib should be treated as the base directory. I learned this the hard way! The current resolution algo is based on Node's, basically.

The extension thing is also a bit tricky because of libraries like https://www.npmjs.com/package/highlight.js!

I'm not quite sure if this stuff is spec'd out – anyone?

@ondras

This comment has been minimized.

Copy link
Author

ondras commented Sep 3, 2015

There's no reliable way to know which directory in myOtherLib should be treated as the base directory. I learned this the hard way! The current resolution algo is based on Node's, basically.

I would say that your "y" shall be imported via

import y from './y.js'

myOtherLib has zero information about its location inside the directory structure, so it has to rely on relative names. Application code, on the other hand, can (and should) leverage the concept of a resolution root.

How are browser implementors going to handle this, once ES6 modules get natively implemented? I would say that the page's BaseURL is going to be the resolution root, completely evading any node_modules automagic (as useful as it is within the current ecosystem). And I would like my client side code, obviously, to work with both rollup and the native ES6 module implementation (future browsers).

As far as extensions go, people are now encouraged to include them: ModuleLoader/es-module-loader#412 (comment)

@Rich-Harris

This comment has been minimized.

Copy link
Contributor

Rich-Harris commented Sep 3, 2015

Drawing a distinction between app code and lib code makes sense – perhaps if we had an option that was called appBaseDir or something people would be discouraged from using non-relative paths inside a library.

For browsers I'd assumed it'd be a case of using the System object's map config, so that the same code could run in front or back with a few config changes. Though I'll admit I hadn't spent too much time thinking about that.

That's interesting re .js extensions. Hmm. I think they work already, perhaps we should actively discourage extension-less paths. Not sure how best to handle cases like highlight.js where the extension is part of the package name.

@Victorystick

This comment has been minimized.

Copy link
Member

Victorystick commented Sep 3, 2015

Oh, highlight.js! Yes, that just got annoying.

import hljs from 'highlight.js';

Requiring extensions in the import paths is a good thing, in my opinion, since it's no longer ambiguous what file is supposed to be imported.

@ondras

This comment has been minimized.

Copy link
Author

ondras commented Sep 3, 2015

Drawing a distinction between app code and lib code makes sense – perhaps if we had an option that was called appBaseDir or something people would be discouraged from using non-relative paths inside a library.

The problem I see here is a library located at node_modules/lib1 that wants to import stuff from its own dependency, located at node_modules/lib1/node_modules/lib2. There is no straightforward ES6 import syntax to accomplish this, provided we (say in a browser) consider the existence of an import root:

import lib2 from "lib2/main.js" // fails

For browsers I'd assumed it'd be a case of using the System object's map config, so that the same code could run in front or back with a few config changes. Though I'll admit I hadn't spent too much time thinking about that.

Right, configuring the System object is probably a way to make stuff work on both front and back. But without adjusting all individual files and prefixing import "util/xxx" with import "../../../util/xxx"! 😄

That's interesting re .js extensions. Hmm. I think they work already, perhaps we should actively discourage extension-less paths. Not sure how best to handle cases like highlight.js where the extension is part of the package name.

Encouraging extension usage seems good to me; at least it aligns Rollup with es6-module-loader. Forced extensions allow us to implement the following universal module resolution algorithm, that covers all mentioned use cases:

  1. does the module id start with a slash? It is an absolute module file name. (/home/ondras/module.js)
  2. does the module id start with a dot? It is a path relative to the importer path. (./net/rpc.js)
  3. (slash-less, dot-less modules)
    1. if the module id corresponds to an existing file name, relative to the import root, use that file (util/net/rpc.js, util/highlight.js/highlight.js)
    2. ELSE the module id is a (node, pmjs) package name; the node_modules resolution procedure shall be started (lib2, highlight.js)

But I am afraid that the described procedure is too complex to be considered :-(

@pakastin

This comment has been minimized.

Copy link

pakastin commented Oct 13, 2015

Let's say I'm writing a view library, called frzr. Why can't I just use import {View} from 'frzr' like I would if I installed frzr to another project?

I mean could the project name itself refer to the root of the project?

@Victorystick

This comment has been minimized.

Copy link
Member

Victorystick commented Oct 13, 2015

@pakasin You should be able to do this now. What error message do you get if you try to do so?

@pakastin

This comment has been minimized.

Copy link

pakastin commented Oct 14, 2015

Could not find package frzr

If I try for example replace this line with:
import {View} from 'frzr'

@Victorystick

This comment has been minimized.

Copy link
Member

Victorystick commented Oct 14, 2015

I see. You could implement your own external resolver (outdated) to handle that.

@indus

This comment has been minimized.

Copy link

indus commented Dec 17, 2015

@Victorystick your hash link is pointing nowhere specific!?!

@Victorystick

This comment has been minimized.

Copy link
Member

Victorystick commented Dec 17, 2015

@indus Sorry, the Wiki has been updated since Oct 14. See the Plugins page instead.

@indus

This comment has been minimized.

Copy link

indus commented Dec 17, 2015

I think it is still super hard to find the function...

rollup({
    plugins: [{
        resolveId: function (code, id) {
            if (id)
                return "some/path/" + code + ".js";
        }
    }]
})

... if you just want to do some simple mapping

@Victorystick

This comment has been minimized.

Copy link
Member

Victorystick commented Dec 17, 2015

It used to be just an option to Rollup. After several iterations, it found it's way into plugins. It's just one level further down, and makes it easier to reuse resolver functions, as well as delegate to others.

rollup({
  plugins: [
    yourResolverPlugin({ ... }),
    anotherPluginThatCouldResolveIds()
  ]
})
@darlanalves

This comment has been minimized.

Copy link

darlanalves commented Feb 6, 2016

First trying rollup in a project and just bumped into the same relative path issues here. So I wrote a plugin to search for files in specific paths inside a project.

https://github.com/dot-build/rollup-plugin-includepaths

It's just a silly recursive search that goes through the defined include paths and checks for file existence there, but worked so far.

@Rich-Harris Thanks for the awesome work on rollup! The final bundle code seems quite clean and organized :)

@Rich-Harris

This comment has been minimized.

Copy link
Contributor

Rich-Harris commented Feb 6, 2016

@darlanalves nice! I've added it to the wiki – thanks

@istr

This comment has been minimized.

Copy link

istr commented Feb 8, 2016

@darlanalves would you mind publishing it to the npm registry? Thanks.

EDIT: sorry... It is already there. 👍 Just github tags are missing. Nice plugin.

@Rich-Harris

This comment has been minimized.

Copy link
Contributor

Rich-Harris commented Jun 5, 2016

Closing as it's been a while since there was any activity on this and I don't think there's anything specific that needs to be done

@jdalton

This comment has been minimized.

Copy link

jdalton commented Jan 12, 2017

@Victorystick

It used to be just an option to Rollup. After several iterations, it found it's way into plugins. It's just one level further down, and makes it easier to reuse resolver functions, as well as delegate to others.

rollup({
  plugins: [
    yourResolverPlugin({ ... }),
    anotherPluginThatCouldResolveIds()
  ]
})

It looks like the first resolver plugin, in your example yourResolverPlugin, may take priority over anotherPluginThatCouldResolveIds. So while yourResolverPlugin gets every module id the anotherPluginThatCouldResolveIds ends up getting none. At least that's what I'm seeing with:

plugins: [
  nodeResolve(),
  { resolveId(a,b) { console.log('B', a, b); return a } },
]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment