search

jetheredge / SquishIt

Lets you *easily* bundle some css and javascript! Check out the Google group if you have questions!
http://groups.google.com/group/squishit
MIT License
459 stars 119 forks source link

Loosen System.Web Stranglehold #315

Open AlexCuse opened 9 years ago

AlexCuse commented 9 years ago

Hide everything that depends on System.Web behind an interface, allow user to specify alternate implementations.

Probably a good change for v1.0, take the opportunity to overhaul configuration API to make it more discoverable.

BundleCache / RawContentCache and DebugStatusReader are the two remaining that come to mind, though I'd imagine there are others.

It'd be nice to put implementations of these interfaces into a new project/package for nancy users, though I am not sure I have the expertise with nancy to figure this stuff out (CacheDependency is an especially glaring need).

AlexCuse commented 8 years ago

I think a new assembly will be needed for the aspnet specific bits but we can include in the current squishit package to minimize disruption. A .nancy package could then include alternate implementations, along with the framework assembly.

Now that we're on .net 4 can make these satellite assemblies register themselves with the framework (like the pre processors do).

Worthaboutapig commented 8 years ago

Is the SquishIt.Mvc not the appropriate place to move the ASP.NET-specific stuff? Or do you support WebForms? Do we not need a SquishIt.Nancy assembly too?

Are you happy for me to pull out the necessary interfaces, or do you want to set up the initial work?

Worthaboutapig commented 8 years ago

I'm put something together as a start, I'll create a PR for you to have a look. Try and have it done in the next couple of days, but might not have much time tomorrow ;)

AlexCuse commented 8 years ago

MVC package is just razor helper stuff, I've never actually used it. I'm thinking a SquishIt.AspNet assembly that gets included with current package will be the way to go. Then if you get nancy implementations together we can put out SquishIt.Nancy and include a pointer in the release notes for any nancy users.

Yeah if you want to pull out the interfaces thats fine. Just create some folder in the Framework project for now and we can get it moved out to a new assembly when ready.

Definitely thinking this should be included in 1.0 release since its a pretty major change.

You can fork from here: https://github.com/AlexCuse/SquishIt/tree/feature/v1.0

Worthaboutapig commented 8 years ago

Thanks. Now have to figure out the best way to fork it, as I can't. I've already forked the jetheredge/SquishIt, so GitHub won't allow me to refork the AlexCuse version, it just points me back to the jetheredge version. And that doesn't have the branches. Could delete my original fork, but don't really want to, or setup another account, but don't want to do that either :S.

(see http://stackoverflow.com/questions/12338240/how-can-i-fork-the-original-repo-when-ive-already-forked-a-different-fork?lq=1)

Any suggestions?

edit: Actually, is there a specific reason the 1.0 branch is not in the parent repository? Then I wouldn't have a problem ;)

Worthaboutapig commented 8 years ago

Ok, I've managed to manually create a fork... however, after switching to your feature/v1.0, I have 40 failed tests. Is this expected?

They're all related to the expected minified script being function product(n,t){return n*t}function sum(n,t){return n+t}\n, whereas it's coming out as function product(c,d){return c*d}function sum(c,d){return c+d}\n e.g. in SquishIt.Tests.JavaScriptBundleTests.CanBundleArbitraryContentsInRelease

AlexCuse commented 8 years ago

Oh man sorry about that - just committed a change flipping the default minifiers back to ajaxmin. I got cold feet about that change in light of #323.

AlexCuse commented 8 years ago

re self registering assemblies (when SquishIt.AspNet gets split out):

https://github.com/AlexCuse/SquishIt/commit/4c4b90568a18ee611ce17621432b23da39cc983e

Worthaboutapig commented 8 years ago

@AlexCuse Could you take a look at the PR I sent you: https://github.com/Worthaboutapig/AC-SquishIt/pull/2

AlexCuse commented 8 years ago

I don't understand that PR - what does it have to do with extracting dependencies on System.Web? Are there unpushed commits?

I am not sure I want to make this change, because the sass project seems to be working for some people and it might break it. I have been watching libsass.net and there seems to be movement in making it "AnyCPU" ready.

AlexCuse commented 8 years ago

Oh wait I see, its PR 1 in your repo not 2.

I never got a notification for this - it looks like you submitted the PR against your repo's feature/v1.0 branch instead of upstream. It does look more or less like what I had in mind though.

Don't worry too much about coding style, though I appreciate your efforts. The project has been around for a while, and its had contributions from a lot of people. I can never quite get a satisfactory result from resharper's "solution wide" cleanup. I especially appreciate your inclusion of xml comments, as I am on the verge of starting to include them in nuget packages for http://dotnetapis.com. Now that this is getting close I will probably wait until we have the new nuget packages defined so I can include them everywhere at once.

Let me know how everything goes, and if I can help with anything. I think you can get my email from the git logs.