javascript_include_tag "extname" option for use by asset_path #11707
Conversation
| @@ -26,7 +26,8 @@ module AssetTagHelper | |||
| # to <tt>assets/javascripts</tt>, full paths are assumed to be relative to the document | |||
| # root. Relative paths are idiomatic, use absolute paths only when needed. | |||
| # | |||
| # When passing paths, the ".js" extension is optional. | |||
| # When passing paths, the ".js" extension is optional. If you do not want .js | |||
There was a problem hiding this comment.
# When passing paths, the ".js" extension is optional. If you do not want ".js"
|
Thanks for the review rafaelfranca. I think I've fixed all the issued you'd highlighted. |
|
Seems great. Could you add a CHANGELOG entry and squash your commits? |
|
Hmm.. Did I perform the rebase correctly? It's strange that the pull request now has all those other commits from others in it now - I've never encountered that behavior before. I didn't pull from upstream rails/rails so how did my branch contain them? Should I abort this pull request and start a new branch based off rails/master? |
You certainly don't need to do that. a8ab4d7 is the commit you want, right? Try this, assuming $ git fetch upstream
$ git checkout extname_option
$ git reset --hard upstream/master
$ git cherry-pick a8ab4d7
$ git push -f origin extname_optionDoes that make sense? |
ActionView::Helpers.asset_path is where the logic for javascript_include_tag resides. It takes an extname option for specifying the extension or false to not append it. This exposes that option to javascript_include_tag. Without the option files that didn't end with ".js" would get the extension appended to them. This broke JST templates and other file types that should be interpreted as JavaScript but who's file extension isn't ".js"
|
Awesome! Thanks Steve - "cherry-pick" to the rescue. I was going to delete my "extname_option" branch and start a new one with the same name, but wasn't sure if it'd still target this pull request if I did so. |
|
That would have worked too. :) |
javascript_include_tag "extname" option for use by asset_path
For now work around by by making our own tag. Pull request rails/rails#11707 will correct on next version of Rails
For now work around by by making our own tag. Pull request rails/rails#11707 will correct on next version of Rails
- goes back to using actual rails helpers instead of cooking up a `script` tag manually, which is frankly crazy - some relevant links: documentcloud@dcc9b02 is where they changed it, which was committed to fix the rails bug at rails/rails#11707. All this is irrelevant for rails 2.3
javascript_include_tag uses ActionView::Helpers.asset_path to compute the path for javascript files. However asset_path automatically appends an extension onto the path which is controlled by the extname option.
javascript_tag_helper only passed the 'protocol' option to asset_path, making it impossible to modify the extname behavior. Without this, underscore JST template files are getting a '.js' extension appended to them.
This breaks the 'jammit' gem since it calls javascript_include_tag internally.
For instance
javascript_include_tag('core.jst')is returning<script src="/javascripts/core.jst.js"></script>