Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AssetAliasUsed should not affect Sass Helpers #247

Closed
josh opened this issue May 12, 2015 · 10 comments
Closed

AssetAliasUsed should not affect Sass Helpers #247

josh opened this issue May 12, 2015 · 10 comments
Assignees

Comments

@josh
Copy link
Contributor

josh commented May 12, 2015

AssetAliasUsed should only be raised from ActionView helpers, not within static assets themselves.

I think this is only a probably with the 2.x backport. 3.x already behaves as intended.

#246
primer/octicons#50

/cc @rafaelfranca

@rafaelfranca
Copy link
Member

The reason is that we include the helpers inside the assets context, so we would have to do something like 32a58f5

@josh
Copy link
Contributor Author

josh commented May 12, 2015

Yeah, for 2.x, I think we just want to do a quick feature check somehow and only apply it to AV. Maybe see if we're is_a?(ActionView::Base)?

@rmosolgo
Copy link

Hey there, just wanted to double-check that I'm experiencing the desired effect here.

react-rails setup

In react-rails, app config specifies either production or development, based on that, we add a path to app.config.assets.paths, roughly speaking:

# add directory to asset pipeline, 
# based on the app config
app.config.assets.paths << root_path.join('lib/assets/react-source/').join(directory).to_s

(full src)

We do this so that the app only needs //= react, then, each environment gets its version of React.js (the development version has nice error messages, the production version has none).

error

However, we get an AssetAliasUsed from the javascript_include_tag helper:

image

Sure enough, these don't match:

image

Interestingly, if I upgrade to Sprockets 3 + sprockets-rails 2.3.1, I don't get this error (which is fine for me, but not fine for many other react-rails users...).

Is this the expected outcome? Are we misusing the asset pipeline? Anything else I can do to diagnose or address this?

We're really hoping to support an API where //= require react can provide different versions of the file based on environment configs. Previously we copied JS files into local directories, but that had its own issues!

Thanks!

@aripollak
Copy link

It seems like this gem now needs a dependency on sprockets >= 3.0, which maybe breaks SemVer?

@rafaelfranca
Copy link
Member

The master branch yes, the released branch no. It works with both version.

@rmosolgo
Copy link

Ok, can you help me understand that error? I don't know why our use of that Environment was unsafe (or how to rework our process)

@rafaelfranca
Copy link
Member

@rmosolgo the check is gone at master branch, so I think we need to revert it in 2.x and release 2.3.2. @josh confirm?

@rmosolgo
Copy link

Any word on 2.3.2 ?

I'm hoping to get out a new version of react-rails but I don't want to publish it while it's incompatible with the latest sprockets-rails 2.x!

I can try to help, but I don't quite understand what needs to be done. Should I just remove the whole check & error?

@rafaelfranca
Copy link
Member

@rmosolgo could you open a new issue so we can keep track of this and not forget to release?

@rmosolgo
Copy link

happy to!

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

Successfully merging a pull request may close this issue.

4 participants