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

Added the ability to require dependencies relative to the cwd #324

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
6 participants
@balupton
Copy link
Contributor

commented Mar 13, 2013

We have a fairly complicated project tree and we'd like to be able to require things with browserify relative to the cwd. So instead of writing require('../../../../../app/out/models/mockup.js') which is prone to change, we can write require('app/out/models/mockup.js') instead which is far less prone to change.

Another way to do this (which could be preferable) is:

// in our code
module.paths.push(process.cwd());
// in browserify
parent.paths = parent.paths.concat(module.paths);

or via an option.

It seems something like this is intended with https://github.com/balupton/node-browserify/blob/7ae499aa71f23d146af9d2dbd4e2f41403c9e218/index.js#L50 but I may be wrong about that.

@substack

This comment has been minimized.

Copy link
Collaborator

commented Mar 13, 2013

This diverges from how node's require() works works in a counter-intuitive way that I would rather not support. It's actually pretty useful that require('../../../../../app/out/models/mockup.js') is very annoying because something fundamental is amiss here. If you just move app/ into node_modules/app instead of this patch, everything will work as you want it to with no changes to browserify necessary.

@substack substack closed this Mar 13, 2013

@dominictarr

This comment has been minimized.

Copy link
Collaborator

commented Mar 13, 2013

you can even symlink it

cd node_modules
ln -s ../ app
cd ..

then require('app') will get you your app.

@Raynos

This comment has been minimized.

Copy link
Contributor

commented Mar 13, 2013

I have uris like ../../../../../app/out/models/mockup.js I like them. It reminds me that my application file structure is completely 100% fucked beyond repair and I need to fix it ASAP.

Right now we are on an aggressive crusade to move all app specific logic into node_modules and build a beatiful recursive tree representing our actual app complexity with each subsection being, isolated, documented and tested independently.

@balupton

This comment has been minimized.

Copy link
Contributor Author

commented Mar 13, 2013

Went with @dominictarr's symlink approach, works well. Cheers for the feedback everyone, makes sense.

@dominictarr

This comment has been minimized.

Copy link
Collaborator

commented Mar 13, 2013

make sure you check in that symlink

git add node_modules/app -f
git commit -m 'link to self'

or people who clone your repo will get confused.

@Raynos

This comment has been minimized.

Copy link
Contributor

commented Mar 13, 2013

@dominictarr can you actually check symlinks in and not have it blow up in windows?

@medikoo

This comment has been minimized.

Copy link

commented Mar 13, 2013

@Raynos put it really well.. I had same issue, it just means that you're code is not modularized well and needs refactor.

@junosuarez

This comment has been minimized.

Copy link
Contributor

commented Mar 13, 2013

Guys, I know some of us has strong opinions about software, but we're not really helping people want to move to node by obstinately telling people they're doing it wrong. It's totally valid for @substack to close the issue, and he did a good job explaining why - having browserify diverge from node's module resolution would be bad. @dominictarr offered a helpful workaround. The rest of this thread just seems unnecessarily prescriptivist.

@balupton

This comment has been minimized.

Copy link
Contributor Author

commented Apr 2, 2013

Hrmmm, turns out the symlink approach doesn't work too well on heroku... via checking in with git, and creating manually via a startup script.

@Raynos @medikoo not sure I particuraly agree... our app is a commercial backbone.js application, which we are attempting to share models between the server and client side (hence the app and site directories). Now, I could make each model it's own npm package, have it's own private git repo, setup the private user permissions, and all that jazz, but that is a getting a bit extreme as our models are changing a lot in this intial stage. One of the unix philosophies is prototype first, then figure out the stuff later.

@substack could we implement this change as:

parent.paths = parent.paths.concat(module.paths);

Which would make browserify behave like node.js's require, that is if we add paths to module.paths then node's require picks it up, however currently browserify does not. Doing that change would make it so browserify does.

This change would mean there is no unexpected or non-standard behavour like the original pull request (using cwd), and would bring more conformity across node and browserify which appears to one of the project's goals.

@junosuarez

This comment has been minimized.

Copy link
Contributor

commented Apr 2, 2013

There's always http://npm.im/relquire

jason

On Apr 1, 2013, at 6:50 PM, Benjamin Arthur Lupton notifications@github.com
wrote:

Hrmmm, turns out the symlink approach doesn't work too well on heroku...
via checking in with git, and creating manually via a startup script.

@Raynos https://github.com/Raynos @medikoo
https://github.com/medikoonot sure I particuraly agree... our app is
a commercial backbone.js
application, which we are attempting to share models between the server and
client side (hence the app and site directories). Now, I could make each
model it's own npm package, have it's own private git repo, setup the
private user permissions, and all that jazz, but that is a getting a bit
extreme as our models are changing a lot in this intial stage. One of the
unix philosophies is prototype first, then figure out the stuff later.

@substack https://github.com/substack could we implement this change as:

// in our codemodule.paths.push(process.cwd());// in
browserifyparent.paths = parent.paths.concat(module.paths);

Which would make browserify behave exactly like node.js's require. If we
add paths to module.path then node's require picks it up, however currently
browserify does not. Doing that change would make it so browserify does.

It would also mean there is no unexpected or non-standard behavour like the
original pull request (using cwd) caused, and bring more conformity across
node and browserify.


Reply to this email directly or view it on
GitHubhttps://github.com//pull/324#issuecomment-15748916
.

@Raynos

This comment has been minimized.

Copy link
Contributor

commented Apr 2, 2013

@balupton we actually have our "models" in private github repos. However not one repo per model, one repo per group.

Setting up private repos is trivial. Setting up private npm is a pain. But really once setup the only difference in workflow between normal and private is where it's going

# normal
scaffold new repo
hub create {{company}}/{{repo}}
npm publish

#private
scaffold new private repo
hub create {{company}}/{{repo}} -p
npm publish

Just make your module scaffolder add the publishConfig for public & private things and use the -p flag to tell github to create a private repo.

But I do feel the pains of trying to work across many repos in terms of workflow, commits & merging.

The most sensible thing might be

/
  /browser
    /models.js
  /server
    /models

Where models.js is module.exports = require("../server/models"). That way you can use relative requires in both frontend & backend code and don't duplicate the models

@dominictarr

This comment has been minimized.

Copy link
Collaborator

commented Apr 2, 2013

module.paths is a legacy feature, and should best be avoided. Even if browserify did support this, following @Raynos's suggestion would be a much better alternative.

@ghost

This comment has been minimized.

Copy link

commented Jan 7, 2014

+1. Creating private github repos seems kind of silly. Same with symlinks. If browserify is going to "truly" bundle a Node.js application for the browser, it should support process.cwd() for sane path structures. IMO, any hack or extra step on top of having browserify handle this is suboptimal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.