Skip to content

Conversation

@tkohlman
Copy link
Contributor

Return a tree object instead of a flat list and provide functions to produce
pre-order and post-order traversals of the tree. The pre-order traversal yields
the same output as the old behavior: a flat list of visited files without
duplicates.

…produce

pre-order and post-order traversals of the tree. The pre-order traversal yields
the same output as the old behavior: a flat list of visited files without
duplicates.
@mrjoelkemp
Copy link
Collaborator

Great work here!

I can't give a proper review until the weekend (just had a baby yesterday!). Sorry about that.

Thanks for contributing.

Btw, saw your dependency-tree-css lib and wouldn't mind thinking of ways of executing a callback per node during traversal. Would that allow you to use dependency-tree as a depenedency instead of duplicating a lot of the logic?

@tkohlman
Copy link
Contributor Author

Congratulations on the baby!

dependency-tree-css was a quick-and-dirty fix to a problem I was having in my Gulp builds. Basically, I wanted a way to find the URL dependencies of a CSS file so that I could copy a minimal set of image and font files to the distribution folder.

I am not sure how easy it would be to merge that functionality to node-dependency-tree, because I added streaming support to dependency-tree-css. Streaming support allows me to pipe the output of something like gulp-less to dependency-tree-css in order to copy the needed files. I process the resulting CSS stream from gulp-less and resolve paths relative to the original LESS file(s). That use case will be the biggest obstacle to overcome.

However, if node-dependency-tree can resolve Less and/or Sass imports, then I probably would not need streaming support. If we can design something functional, I would be happy to implement it and deprecate dependency-tree-css.

@mrjoelkemp
Copy link
Collaborator

Thanks!

Sass support should be there if dependency-tree uses my module
node-precinct to extract dependencies. Less support is a matter of further
generalizing my detective-sass module or rolling a custom detective-less.
On Feb 18, 2015 3:05 PM, "Tom Kohlman" notifications@github.com wrote:

Congratulations on the baby!

dependency-tree-css was a quick-and-dirty fix to a problem I was having in
my Gulp builds. Basically, I wanted a way to find the URL dependencies of a
CSS file so that I could copy a minimal set of image and font files to the
distribution folder.

I am not sure how easy it would be to merge that functionality to
node-dependency-tree, because I added streaming support to
dependency-tree-css. Streaming support allows me to pipe the output of
something like gulp-less to dependency-tree-css in order to copy the needed
files. I process the resulting CSS stream from gulp-less and resolve paths
relative to the original LESS file(s). That use case will be the biggest
obstacle to overcome.

However, if node-dependency-tree can resolve Less and/or Sass imports,
then I probably would not need streaming support. If we can design
something functional, I would be happy to implement it and deprecate
dependency-tree-css.


Reply to this email directly or view it on GitHub
#18 (comment)
.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had no real intention behind doing a pre-order traversal. Perhaps all of this could be masked behind dependencyTree.asList(tree) that returns a postOrder list.

@mrjoelkemp
Copy link
Collaborator

@tkohlman great work. I'm going to change things up slightly:

  1. change the object form to using filepaths as indexes to achieve a format desired for New Feature: Open dependency tree for a module as a JSON file Dependents#55. I'm open to arguments against this format.
  2. Make post-order the default traversal method and change the api to asList.

I'll keep most of your code and tests and give you credit for the commit. Thanks for contributing. This is awesome.

mrjoelkemp pushed a commit that referenced this pull request Feb 24, 2015
Also includes CLI flags for future extension.

Fixes #16
Fixes #17
Closes gh-18
mrjoelkemp pushed a commit that referenced this pull request Feb 24, 2015
Also includes CLI flags for future extension.

Fixes #16
Fixes #17
Closes gh-18
@mrjoelkemp
Copy link
Collaborator

@tkohlman Thanks again for the help. I made the tweaks and cut v3.0.0. Check out the readme for the updates.

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.

2 participants