javascript_include_tag + duplicated files #31

Closed
jejacks0n opened this Issue Jan 1, 2013 · 6 comments

Comments

Projects
None yet
3 participants
@jejacks0n
Contributor

jejacks0n commented Jan 1, 2013

Please let me know how I can proceed to best help.

I came across an issue that exists in Sprockets::Helpers::RailsHelper.javascript_include_tag (Rails 3.2.9) where in files can be required and loaded multiple times. Calling flatten on the array before uniq resolves this issue. Review an example.

I'll add a commit that resolves this in 3.2, but since all of this is changing and in (what appears to be) heavy flux for 4, I'm unsure how I can approach ensuring that it doesn't linger in the next release. I'm aware that these helpers will go away, and that's where I'm looking for guidance.

@rafaelfranca

This comment has been minimized.

Show comment Hide comment
@rafaelfranca

rafaelfranca Jan 1, 2013

Member

What helpers will go away?

Before fixing on 3.2 you need to fix the issue for the Rails 4. Since all the sprockets integration was moved to this repository in the Rails 4 you need to fix the helpers here.

Member

rafaelfranca commented Jan 1, 2013

What helpers will go away?

Before fixing on 3.2 you need to fix the issue for the Rails 4. Since all the sprockets integration was moved to this repository in the Rails 4 you need to fix the helpers here.

@jejacks0n

This comment has been minimized.

Show comment Hide comment
@jejacks0n

jejacks0n Jan 1, 2013

Contributor

I know -- that's why I created the issue here, and not in rails/rails. =)

So, my question is how you would like me to proceed in fixing it.. it's considerably different, so while the code was moved from actionpack/sprockets, it's different enough that I wanted to see if anyone had suggestions on it -- which is when I came across the other issue that you closed. =) without understanding that, it's hard to even get rails 4 to load assets from a mountable engine.

Contributor

jejacks0n commented Jan 1, 2013

I know -- that's why I created the issue here, and not in rails/rails. =)

So, my question is how you would like me to proceed in fixing it.. it's considerably different, so while the code was moved from actionpack/sprockets, it's different enough that I wanted to see if anyone had suggestions on it -- which is when I came across the other issue that you closed. =) without understanding that, it's hard to even get rails 4 to load assets from a mountable engine.

@jejacks0n

This comment has been minimized.

Show comment Hide comment
@jejacks0n

jejacks0n Jan 1, 2013

Contributor

also, the javascript_include_tag style helpers are commented to be going away -- replaced by source maps...

https://github.com/rails/sprockets-rails/blob/master/lib/sprockets/rails/helper.rb#L85

Contributor

jejacks0n commented Jan 1, 2013

also, the javascript_include_tag style helpers are commented to be going away -- replaced by source maps...

https://github.com/rails/sprockets-rails/blob/master/lib/sprockets/rails/helper.rb#L85

@rafaelfranca

This comment has been minimized.

Show comment Hide comment
@rafaelfranca

rafaelfranca Jan 1, 2013

Member
@jejacks0n

This comment has been minimized.

Show comment Hide comment
@jejacks0n

jejacks0n Jan 2, 2013

Contributor

Wish I knew how to link a pull request to this issue.

#33

Contributor

jejacks0n commented Jan 2, 2013

Wish I knew how to link a pull request to this issue.

#33

guilleiguaran added a commit that referenced this issue Jan 10, 2013

Merge pull request #33 from jejacks0n/duplicate_assets
Ensure assets aren't duplicated for debug. #31
@frodsan

This comment has been minimized.

Show comment Hide comment
@frodsan

frodsan May 3, 2013

@rafaelfranca This can be closed.

frodsan commented May 3, 2013

@rafaelfranca This can be closed.

junaruga added a commit to junaruga/sprockets-rails that referenced this issue May 17, 2017

junaruga added a commit to junaruga/sprockets-rails that referenced this issue May 17, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment