Skip to content
This repository has been archived by the owner on Aug 10, 2020. It is now read-only.

Inject analytics javascript using Rack #24

Closed
jellybob opened this issue Sep 21, 2011 · 12 comments
Closed

Inject analytics javascript using Rack #24

jellybob opened this issue Sep 21, 2011 · 12 comments
Labels

Comments

@jellybob
Copy link
Contributor

Just a quick idea, which I may or may not have time to hack on at some point. It would be really neat if I could avoid having to add all the [head|body]_[prepend|append]_javascript calls in my layout by having a Rack middleware which injects them as the response goes out.

@jkrall
Copy link
Owner

jkrall commented Sep 21, 2011

This is a great idea. Off the top of my head, I can't think of any reason why it wouldn't work... If you do have time, I'd love to have some help with it.

To be honest, I have mixed feelings about rack middleware that messes with the actual page markup. Part of me feels weird about having the rendered page modified on its way out to the browser. However, I do agree that the prepend/append just is clutter in our layouts.

I think It would be great to have as a option, maybe enabled by default.

On Sep 21, 2011, at 6:18 PM, Jon Woodreply@reply.github.com wrote:

Just a quick idea, which I may or may not have time to hack on at some point. It would be really neat if I could avoid having to add all the [head|body]_[prepend|append]_javascript calls in my layout by having a Rack middleware which injects them as the response goes out.

Reply to this email directly or view it on GitHub:
#24

@nirvdrum
Copy link
Collaborator

My guess is this is going to be really tricky to get right. Unless I'm grossly mistaken, your two options are parse with a regexp or load in with Nokogiri or some such. Both are error prone in the general case. Additionally, that's some pretty hefty overhead on response time. So I'm -1 on this.

If you do add it please retain the old helpers.

@sfsekaran
Copy link
Collaborator

True, but there's always deface (https://github.com/spree/deface), whose job it is to inject stuff into views.

@nirvdrum
Copy link
Collaborator

Then I guess those that want to inject should look at using deface? It still looks like a terrible idea, even if there is an entire gem dedicated to it. And taking a quick walk through that code, the gem is anything but lightweight.

@sfsekaran
Copy link
Collaborator

Agreed. We shouldn't include deface here. But maybe we simply provide an app/overrides/analytical.rb so that if someone does include it in their gemfile, it gets loaded automatically?

BTW, what's interesting about deface is that it uses a precompile phase. On production, it only runs the processed files, and doesn't re-run the defacement every request.

@nirvdrum
Copy link
Collaborator

It sounds like a lot of complexity to avoid placing 4 lines into a layout file. Maybe I'm missing something, but even if you wanted to do that, why not just set up deface and not add these lines into your layout? Presumably it's a general purpose tool all the analytical helpers do is generate text (i.e., there's nothing special requiring their use in a layout).

@sfsekaran
Copy link
Collaborator

True. And I tend to agree with you. But if someone sends a pull request for an example deface file or some readme text regarding deface integration, I think we should accept it. Regarding rack injection--I agree that it would have to be off by default for me to feel comfortable with it.

@nirvdrum
Copy link
Collaborator

Well, I'm not a collaborator (although I offered to take over the project), so I don't have much say. But if I did, I'd be fine with a wiki page. Beyond that, it really should be a separate analytical-deface project, IMHO. I don't think this is remotely something we should be encouraging, I don't think deface is popular enough to merit special inclusion, and I don't think the project really should expand to fill all possible usage scenarios. The project should focus on its core competencies, which unfortunately are currently broken in master -- mostly an artifact of just merging in well-intentioned, but far reaching, pull requests.

@sfsekaran
Copy link
Collaborator

Fair enough, let's close this request for now. If anyone would like to argue otherwise, we can reopen it.

@jkrall
Copy link
Owner

jkrall commented Feb 16, 2013

Appreciate the discussion here guys, I agree with the conclusion.

@nirvdrum FWIW... I would be glad to add you as a collaborator, if you would like to be more involved.
I don't remember your request to take over the project, but for now, it seems like there's a healthy level of maintenance going on... so no bigger changes are required. Ping me if you disagree?

I believe master is being fixed in another discussion, @sfsekaran has been working on that, afaik.

@nirvdrum
Copy link
Collaborator

@jkrall No worries. Since GitHub removed the ability to directly email people, I reached out over Twitter a little over a month ago: https://twitter.com/nirvdrum/status/288734677823541249 I think I had tried a few months before that, but can't dig up the tweet. Obviously not the best form of communication.

I'm happy with the maintenance work picking up and do apologize if my frustrations came off as either personal attacks or unappreciation. Obviously I was a bit aggravated with master having some necessary bugfixes, but simultaneously being broken. But, I'm also all too familiar with the struggle of maintaining a project you may no longer use or don't have the time for. So, I'm happy to help out any way I can. And I'm just as happy to not take over another project :-)

@jkrall
Copy link
Owner

jkrall commented Feb 18, 2013

@nirvdrum It's cool... no offense taken, and I certainly understand the frustration with the project becoming broken & stagnant.

Also, sorry, I haven't really used twitter in months! To be honest, it never occurred to me that since github remove messaging it wouldn't be possible to reach me. Feel free to ping me anytime at joshuakrall at pobox.com.

I've added you as a contributor... I'm sure everyone here would love any help you are willing to give. If you decide you don't have time or don't want your name attached, no big deal.
Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants