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

delay loading dependencies until task execution #88

Merged
merged 1 commit into from
Jul 20, 2014

Conversation

c089
Copy link
Contributor

@c089 c089 commented Nov 7, 2013

bower is extremely slow to require(), therefore just loading the bower task
will masssively slow down a grunt build even if the bower task is not executed
in that particular grunt run

Actually, I realize this is a somewhat crude hack and the issue should be properly fixed in bower (by not wasting a whole second for a require on decent hardware) and maybe also in the grunt API (something like an initialize method that is only run when the task is actually used) - but this annoyed me so much so many times every single day that I had a quick shot at it... Feel free to close and hate me for this PR. Or if you have a better idea for a cleaner workaround, I'd love to hear it ;)

💖, Chris

bower is extremely slow to require(), therefore just loading the bower task
will masssively slow down a grunt build even if the bower task is not executed
in that particular grunt run
@c089
Copy link
Contributor Author

c089 commented Nov 7, 2013

Reported the root cause as bower/bower#958

@yatskevich
Copy link
Owner

Hi Chris,

Thanks for the PR! In fact you've found out a real issue in our task - we're requireing dependencies at initialization time while those dependencies are really needed only at task execution time.

So, one option is to move all of the deps and helper function into the task body. The second option - refactor helper functions to take needed modules/functions as parameters (kind of manual dependency injection) and still move all require calls into task body.

What do you think?

@c089
Copy link
Contributor Author

c089 commented Nov 8, 2013

I'm fine with both. Actually, I quite like the require statements at the top as they were before my PR - it seems to be idiomatic node and it wouldn't be a problem if bowers' require() didn't have those side-effects (whatever they are)...

@jasonkarns
Copy link

I like the idea of moving the require calls into the functions as params. (In CoffeeScript, I would have used default args so the dependency could be overridden by the caller. ES6 may even give us default args natively.) At any rate, I don't like top-loading the require calls. I believe this is really just a pattern emulating languages where require calls are language constructs. Since they are simple function calls in JS, we shouldn't treat them as special. Invoking require as close to its actual dependency is a good thing, IMO.

A strikingly similar conversation is occurring on grunt itself: gruntjs/grunt#975. I'm mainly just reiterating @cowboy's comment that the require calls should be delayed to where they are actually needed.

yatskevich added a commit that referenced this pull request Jul 20, 2014
delay loading dependencies until task execution
@yatskevich yatskevich merged commit b9e7a54 into yatskevich:devel Jul 20, 2014
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