disqus / grunt-smartrev

A "smart" file versioner for production environments which takes inter-file dependencies into account automatically.
Apache License 2.0
75 stars 6 forks source link

Assetgraph collaboration #7

Open Munter opened 10 years ago

Munter commented 10 years ago

It seems smartrev is trying to do the exact same thing as assetgraph already does. We have been at this for years and have probably covered more ground that you have already.

Would you be interested in a pull request that uses assetgraph to do the same work with a lot less code and a well tested library?

The downside is that assetgraph has a few dependencies that are heavier than what the current task has.

Even if there is no ground for collaboration we might have already mapped out dependencies that you haven't gotten around to yet. Feel free to steal all out parsing logic to detect outgoing dependencies.

BYK commented 10 years ago

@Munter - Thanks for the suggestion! assetgraph was one of the things I've looked at which quite amazed me. That said it was doing too much for what we wanted so we we came up with smartrev.

Do you have any specifics for collaboration? I'd love to converge to a common tool instead of having multiple projects for the same or very similar things.

Munter commented 10 years ago

Ideally I'd like to start pulling out some of the relation detections into external npm modules so anyone could consume them. This would be beneficial for all sort of projects that use dependency graphs, like browserify, webpack, systemjs etc. It's however not that easy, as some of the relations we are working with need the graph instance available to figure out their base asset to resolve relative urls from. It makes the whole thing tighter coupled than I'd like, so that one is a bit further future I guess.

Another way of collaborating would be either to switch to use assetgraph as an engine, which would require us to get a bit lighter on the initial dependencies. I'd also like to learn from the experiences of implementations that have a similar, but lighter approach, and bring their simplicity into assetgraph core.

I guess apart from actually going in and replacing smartrevs code with assetgraph I don't have any specific ideas on actionable things to do.

But I'd certainly like to keep in touch so the dependency graph based approach can move ahead faster than what the file based approach to web asset manipulation has done. I'll follow this project for now, and see if I can find the time to dive deeper into your specific implementation and find your gems :)

BYK commented 10 years ago

Awesome. Let's keep in touch. Gonna keep this ticket open in case anyone is interested. We can discuss any specifics you have through e-mail. It's my nickname here @ disqus ;)

Also just want to make sure that I do agree with all your suggestions. Ideally though we'd separate out the engine which both projects would use. The thing I need to figure out soon is handling circular dependencies properly.

Munter commented 10 years ago

I think we have the circular dependency modelling down pretty well. We ran into some trouble when we started modelling http redirects as assets and ran into Githubs gh-pages 302 --> 200 redirect to the same url, but that should be fixed now.

Circular dependencies should obviously be allowed, as they are a natural part of web sites. But post order traversal is not possible on circular subgraphs. The way we solve that part in assetgraph is running moveAssetsInOrder (the transform with functionality similar to smartrev) only on a selected subgraph, excluding the obvious culprits like html anchors binding the pages together in circles.

However, we still just throw an error if we hit a circle while attempting a post order traversal :( https://github.com/assetgraph/assetgraph/blob/master/lib/transforms/moveAssetsInOrder.js#L35

This is the quite elaborate filtering we need to avoid wrong renaming and circular subgraphs: https://github.com/assetgraph/assetgraph-builder/blob/master/lib/transforms/buildProduction.js#L286-L332

Munter commented 7 years ago

We just released assetgraph v3, which adds a whole bunch of new modellings of relations that we didn't have before and grunt-smartrev doesn't have either. Most importantly though: Our dependencies have lightened considerably, no longer requiring any modules that do gyp-compiles!

BYK commented 7 years ago

Hi @Munter, congrats on the new features!

We shouldn't have any gyp-compile requiring dependencies anyways :) Sorry we didn't have time to collaborate yet but I still want to do this. I may have more time in a couple of months. In the meantime, LMK if there are some easy wins we can have. First thing I want to do is actually separate the project from Grunt so it can be used as a command-line tool.