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

Reading from STDIN does not seem to work #64

Closed
jhthorsen opened this issue Jan 15, 2015 · 10 comments · May be fixed by #96
Closed

Reading from STDIN does not seem to work #64

jhthorsen opened this issue Jan 15, 2015 · 10 comments · May be fixed by #96

Comments

@jhthorsen
Copy link

I'm trying to pipe data into module-deps but it fails:

$ cat robot.js | ./node_modules/.bin/module-deps -

events.js:72
        throw er; // Unhandled 'error' event
              ^
Error: path must be a string
    at /home/username/node_modules/module-deps/node_modules/browser-resolve/node_modules/resolve/lib/async.js:15:16
    at process._tickCallback (node.js:419:13)

Running module-deps with "robots.js" as input on the other hand works like a charm:

$ ./node_modules/.bin/module-deps robot.js 
[
{"file":"/home/username/robot.js","id":"/home/username/robot.js","source":"module.exports = function (s) { return s.toUpperCase() + '!' };\n","deps":{},"entry":true}
]

I'm not sure how the output should look when you pipe though... How will it know the path to the entry point? Will it simply have {"file":"-","id":"-",...}

$ nodejs --version
v0.10.33
$ npm info module-deps | grep version:
  version: '3.6.4',
@zertosh
Copy link
Member

zertosh commented May 5, 2015

Weird. I see the option in cmd.js but the code looks like it never supported it. I don't think this is a feature worth supporting though. Do you need it for something?

@ornj
Copy link

ornj commented Aug 3, 2015

I've run into an opportunity where this feature would be helpful. I have a non-node application that is processing a Javascript file through module-deps and I have the contents of the module in memory.

Probably very much an edge case.

@zertosh
Copy link
Member

zertosh commented Aug 4, 2015

@ornj I think the big blocker for me is figuring out how to support multiple input files, and how to name them. I'd rather not implement stdin support at all if it's going to be half-baked. Any ideas?

@ornj
Copy link

ornj commented Aug 4, 2015

I haven't really looked into the source very much so my understanding is pretty shallow. Why do they need to be named? What if there is no file on disk?

Browserify reads from stdin, browserify/browserify#1201 (comment). Maybe some inspiration can be taken from that?

@zertosh
Copy link
Member

zertosh commented Aug 5, 2015

@ornj The file needs to be named because that's how modules are identified. browserify does read stdin and gives files a default name of $BASEDIR/_stream_0.js or what you specify in --file. The file doesn't have to exist, it's just a dummy identifier. I guess the same can be done here. Want to submit a PR?

@ornj
Copy link

ornj commented Aug 5, 2015

I've started to take a look but I'm not a Node dev. I don't have a good environment for debugging Node command line scripts and I'm finding the whole process cumbersome. This doesn't seem like it would be too hard if I could figure out the appropriate place to parse the stream.

As far as supporting multiple files, I don't think that's really necessary. Using @jhthorsen example from above, you're concatenating files so in practice you're really only providing a single file.

zertosh added a commit that referenced this issue Aug 10, 2015
@zertosh
Copy link
Member

zertosh commented Aug 10, 2015

@ornj I'll hopefully be merging #96 in the next few days – that'll fix piping to the CLI

@ornj
Copy link

ornj commented Aug 10, 2015

👍

@jhthorsen
Copy link
Author

Have this issue and the pull request gone stale?

@jhthorsen
Copy link
Author

Seems to have gone stale.

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 a pull request may close this issue.

3 participants