-
Notifications
You must be signed in to change notification settings - Fork 337
Conversation
It's not entirely clear to me what this PR actually does. That being said, it looks like this adds a dependency on a node environment. One of the advantages of Rust for CLI stuff is that you don't need to have a bunch of stuff installed to use it; regardless of other details, this feels like a regression to me. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey @xtuc ! thanks for this- i see what this PR is doing but i think it's something that we will want to do slightly differently. i'd like the vast majority of the logic to exist in wrangler- which is to say i think the logic that currently exists in wrangler-js needs to be rewritten in rust. i would like wrangler to be checking for the presence of node, npm, webpack, ensuring their versions are correct. wrangler should be orchestrating, not wrangler-js. this way we can version check, type check, etc.
to get started on that, i'd like to see the steps and dependencies that wrangler-js performs/needs, be listed out in the issue. from there we can discuss moving the functionality to wrangler!
@steveklabnik you can find more information at https://github.com/cloudflare/wrangler/wiki/Draft-Webpack-design. I agree that having a single binary would be simpler, but given that we will depend JavaScript code, I'm not sure what's the best solution. One thing I have in mind is that I could be opt-in, for Rust based project invoke |
I think it would be easier to have two branches, there are too many moving parts to be in master. |
…to feat-wranglerjs
refs #69
I have a private demo: https://github.com/xtuc/wrangler-tmp, please let me know if you want access.
I would appreciate some early feedback.