Skip to content
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

resolve deps to row.id rather than row.file #88

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zoubin
Copy link

@zoubin zoubin commented May 12, 2015

I encountered the problem in browserify/browserify#1260

I think there is something can be done in module-deps to fix that problem. But I am not sure if it's the right way.

The rows generated by module-deps should contain a dependency graph with the key id (identifying a node in the graph), yet row.deps are resolved to absolute file paths, rather than the ids of the corresponding dependent rows, which means we may not find some dependent row from the deps property.

The rows generated by module-deps should contain a dependency graph with the keyid(identifying a node in the graph). I am sorry to bother, if I misunderstand the deps output.

Here is an example (with some twists to https://github.com/substack/module-deps/blob/master/example/deps.js):

var mdeps = require('../');
var JSONStream = require('JSONStream');

var md = mdeps();
md.pipe(JSONStream.stringify()).pipe(process.stdout);
md.write({ file: './files/main.js' });
md.write({ file: './files/foo.js', id: './files/foo.js' }); // foo.js is required in main.js
md.end();

The output is:

[
{"file":"./files/main.js","id":"/Users/zoubin/usr/src/zoubin/module-deps/example/files/main.js","source":"var foo = require('./foo');\nconsole.log('main: ' + foo(5));\n","deps":{"./foo":"/Users/zoubin/usr/src/zoubin/module-deps/example/files/foo.js"},"entry":true}
,
{"id":"/Users/zoubin/usr/src/zoubin/module-deps/example/files/bar.js","source":"module.exports = function (n) {\n    return n * 100;\n};\n","deps":{},"file":"/Users/zoubin/usr/src/zoubin/module-deps/example/files/bar.js"}
,
{"file":"./files/foo.js","id":"./files/foo.js","source":"var bar = require('./bar');\n\nmodule.exports = function (n) {\n    return n * 111 + bar(n);\n};\n","deps":{"./bar":"/Users/zoubin/usr/src/zoubin/module-deps/example/files/bar.js"},"entry":true}
]

The deps of main.js is {"./foo":"/Users/zoubin/usr/src/zoubin/module-deps/example/files/foo.js"}. It is impossible to retrieve the foo.js row from the output rows. And that is why I got an undefined deps in browserify/browserify#1260

@jmm
Copy link
Collaborator

jmm commented May 12, 2015

@zoubin I haven't looked at your PR in detail yet, but in case you're interested I'm working on some better documentation for how row properties are used throughout the pipeline to try to squash a bunch of these kinds of issues in a coordinated way. Here's what I have right now:
http://jmm.github.io/browserify-pipeline-docs/

@zertosh
Copy link
Member

zertosh commented May 13, 2015

@zoubin I took a quick glance and realized that this is going to require serious thought - Give me a few days to clear up some time for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants