Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

Asset refactoring broke expansions

#6114

Change 6a609dbc82d03eb92a85970aa157192657f14882 broke expansions (this was arrived via git bisect). Now register_javascript_expansion and register_stylesheet_expansion both set each other's values, so because stylesheets are registered last, Rails produces HTML that tries to load stylesheets as if they were JavaScript. Was this commit tested?

Here is some debugging of the current (399730bdd2f133e9fecac501e2f2333be5f29aa2) version using a simplified version of our codebase:

register_javascript_expansion
========================================================================
expansions (parameter):
{:defaults=>["http://ajax.googleapis.com/ajax/libs/jquery/1.4.3/jquery.min.js", "http://ajax.googleapis.com/ajax/libs/jqueryui/1.8.5/jquery-ui.min.js", "jquery-cookie/jquery.cookie", "rails"], :ie=>["http://html5shim.googlecode.com/svn/trunk/html5.js"], :ie6=>["jquery.supersleight.min"], :datatables=>["jquery-datatables/media/js/jquery.dataTables"], :fancybox=>["jquery-fancybox/fancybox/jquery.fancybox-1.3.4.pack"], :multiselect=>["jquery-tmpl/jquery.tmpl.min", "jquery-blockui/jquery.blockUI", "jquery-localisation/jquery.localisation.min", "jquery-multiselect/js/ui.multiselect"]}

javascript expansions (before merge):
{}

stylesheet expansions (before merge):
{}

javascript expansions (after merge):
{:defaults=>["http://ajax.googleapis.com/ajax/libs/jquery/1.4.3/jquery.min.js", "http://ajax.googleapis.com/ajax/libs/jqueryui/1.8.5/jquery-ui.min.js", "jquery-cookie/jquery.cookie", "rails"], :ie=>["http://html5shim.googlecode.com/svn/trunk/html5.js"], :ie6=>["jquery.supersleight.min"], :datatables=>["jquery-datatables/media/js/jquery.dataTables"], :fancybox=>["jquery-fancybox/fancybox/jquery.fancybox-1.3.4.pack"], :multiselect=>["jquery-tmpl/jquery.tmpl.min", "jquery-blockui/jquery.blockUI", "jquery-localisation/jquery.localisation.min", "jquery-multiselect/js/ui.multiselect"]}

stylesheet expansions (after merge):
{:defaults=>["http://ajax.googleapis.com/ajax/libs/jquery/1.4.3/jquery.min.js", "http://ajax.googleapis.com/ajax/libs/jqueryui/1.8.5/jquery-ui.min.js", "jquery-cookie/jquery.cookie", "rails"], :ie=>["http://html5shim.googlecode.com/svn/trunk/html5.js"], :ie6=>["jquery.supersleight.min"], :datatables=>["jquery-datatables/media/js/jquery.dataTables"], :fancybox=>["jquery-fancybox/fancybox/jquery.fancybox-1.3.4.pack"], :multiselect=>["jquery-tmpl/jquery.tmpl.min", "jquery-blockui/jquery.blockUI", "jquery-localisation/jquery.localisation.min", "jquery-multiselect/js/ui.multiselect"]}


register_stylesheet_expansion
========================================================================
expansions (parameter):
{:defaults=>["jquery-ui-1.8.6.g2"], :datatables=>["jquery-datatables"], :fancybox=>["../javascripts/jquery-fancybox/style"], :multiselect=>["../javascripts/jquery-multiselect/css/ui.multiselect", "jquery-multiselect"]}

javascript expansions (before merge):
{:defaults=>["http://ajax.googleapis.com/ajax/libs/jquery/1.4.3/jquery.min.js", "http://ajax.googleapis.com/ajax/libs/jqueryui/1.8.5/jquery-ui.min.js", "jquery-cookie/jquery.cookie", "rails"], :ie=>["http://html5shim.googlecode.com/svn/trunk/html5.js"], :ie6=>["jquery.supersleight.min"], :datatables=>["jquery-datatables/media/js/jquery.dataTables"], :fancybox=>["jquery-fancybox/fancybox/jquery.fancybox-1.3.4.pack"], :multiselect=>["jquery-tmpl/jquery.tmpl.min", "jquery-blockui/jquery.blockUI", "jquery-localisation/jquery.localisation.min", "jquery-multiselect/js/ui.multiselect"]}

stylesheet expansions (before merge):
{:defaults=>["http://ajax.googleapis.com/ajax/libs/jquery/1.4.3/jquery.min.js", "http://ajax.googleapis.com/ajax/libs/jqueryui/1.8.5/jquery-ui.min.js", "jquery-cookie/jquery.cookie", "rails"], :ie=>["http://html5shim.googlecode.com/svn/trunk/html5.js"], :ie6=>["jquery.supersleight.min"], :datatables=>["jquery-datatables/media/js/jquery.dataTables"], :fancybox=>["jquery-fancybox/fancybox/jquery.fancybox-1.3.4.pack"], :multiselect=>["jquery-tmpl/jquery.tmpl.min", "jquery-blockui/jquery.blockUI", "jquery-localisation/jquery.localisation.min", "jquery-multiselect/js/ui.multiselect"]}

javascript expansions (after merge):
{:defaults=>["jquery-ui-1.8.6.g2"], :ie=>["http://html5shim.googlecode.com/svn/trunk/html5.js"], :ie6=>["jquery.supersleight.min"], :datatables=>["jquery-datatables"], :fancybox=>["../javascripts/jquery-fancybox/style"], :multiselect=>["../javascripts/jquery-multiselect/css/ui.multiselect", "jquery-multiselect"]}

stylesheet expansions (after merge):
{:defaults=>["jquery-ui-1.8.6.g2"], :ie=>["http://html5shim.googlecode.com/svn/trunk/html5.js"], :ie6=>["jquery.supersleight.min"], :datatables=>["jquery-datatables"], :fancybox=>["../javascripts/jquery-fancybox/style"], :multiselect=>["../javascripts/jquery-multiselect/css/ui.multiselect", "jquery-multiselect"]}
register_javascript_expansion

If I find a solution I'll try to post a patch.

Reported by Matthew Ratzloff · December 3rd, 2010 @ 09:19 PM

State: resolved
Milestone: 3.1
Assigned to: Piotr Sarnacki Piotr Sarnacki
Importance: Low

Activity

  1. Matthew Ratzloff
    Matthew Ratzloff

    The problem is that both are reading from ActionView::Helpers::AssetTagHelper::AssetIncludeTag.expansions. The easiest way to solve the problem is to give each child class its own expansions class variable.

    This is a hack since AssetIncludeTag#expand_sources references this, but it's a start.

    December 3rd, 2010 @ 09:39 PM

  2. Matthew Ratzloff
    Matthew Ratzloff

    For anyone dealing with this issue, here's a monkey patch file that implements the somewhat hacky (but not risky) patch above. I've placed it under lib and required it at the top of config/application.rb:

    # See https://rails.lighthouseapp.com/projects/8994/tickets/6114
    require File.expand_path('../../lib/6114_patch', __FILE__)
    

    Hope that helps.

    December 3rd, 2010 @ 10:02 PM

  3. Piotr Sarnacki
    Piotr Sarnacki
    • Milestone set to 3.1
    • Assigned user set to Piotr Sarnacki
    • Importance changed from to Low

    December 16th, 2010 @ 07:25 AM

  4. Repository
    Repository
    • State changed from new to resolved

    (from [09195f10bd3add1be1983a42760113ef2bd31602]) Do not use the same hash instance for expansions [#6114 state:resolved]

    Using the same hash instance makes using the same expansions for
    both javascripts and stylesheets.
    https://github.com/rails/rails/commit/09195f10bd3add1be1983a4276011...

    December 22nd, 2010 @ 08:45 AM