rails / sprockets-rails

Sprockets Rails integration
MIT License
575 stars 244 forks source link

sprockets 4 manifest.js location #369

Open dreyks opened 8 years ago

dreyks commented 8 years ago

Is there a plan to make the location of manifest.js customizable?

I have an app that consists from engines exclusively, so there's no app folder at all. There's one non-isolated engine, which serves as an app folder, e.g. it holds the application.html.erb etc.

Right now every engine has its own application.js file, and the application.html.erb has `<%= stylesheet_link_tag engine_name %> where needed

But sprockets 4 requires me to have manifest.js located exactly at app/assets/config

rafaelfranca commented 8 years ago

The manifest can be in any location that is part of the sprockets load path. lib/assets/foo is an example of this path. Is it not working to you?

dreyks commented 8 years ago

It raises ManifestNeededError if it's not there

rafaelfranca commented 8 years ago

Oh, got it, yeah I think we should remove that check cc @schneems

schneems commented 8 years ago

Thinking aloud

I don't see a benifit of making the location of the file customizable. While technically yes you can put that config in any file that will be auto loaded we need some way to know that the project is Sprockets 4 ready. I don't want to ease that requirement unless there's a good reason to do so. That's how I feel about Rails apps, but not engines.

I see a useful error message as, well useful. If we get rid of the error message we need something else to ease people into the upgrade process to Sprockets 4.

I do think we should allow assets in engines/gems to not have to change anything to support the new version. I guess I would prefer that those work out of the box by default and require fewer changes (if indeed they do work out of the box).

Maybe we could make the check for that file happen only after a precompile check failure?

dreyks commented 8 years ago

If the main reason for a useful message is to ease the transition, maybe there could be some sort of a silencer. Like "we couldn't find manifest.js in the correct location, please put it there. If you know what you're doing, set disable_manifest_strict_check = true to suppress this check"

moving this check to "after-fail" is even cooler since it doesn't require any additional steps

schneems commented 8 years ago

If the main reason for a useful message is to ease the transition, maybe there could be some sort of a silencer

The silencer in this case is the presence of the file ;)

moving this check to "after-fail" is even cooler since it doesn't require any additional steps

Yeah, i like this approach. Still error but only when a precompile asset isn't found. The only downside is that it becomes a runtime error instead of a boot time error. i.e. people may not hit the problem in dev (if they never go to a specific page that doesn't have a specific asset). Still I think this would be the best approach.

richpeck commented 7 years ago

Did this ever get resolved? I have a Rails engine for which I am unable to declare the manifest.js file and need to know where to place it

philss commented 7 years ago

I had the same issue here. I was trying to use Sprockets 4 in a Rails engine without success.

After some experiments, I could make it work by creating an empty manifest.js file in the host app, which in my case was the dummy app.

Also I had to create the engine's manifest and include this manifest path as an additional precompile file in the engine config file.

The manifest file and the engine config can be found in this POC.

Considering that configuring the engine make it works, should Rails add that configuration by default for the rails new plugin generator? Or should the docs for sprockets-rails explain this? Do we have the option to include an explanation about engines in this exception message?

Thanks!

richpeck commented 7 years ago

I actually fixed this pretty well. It works on Sprockets 3x and 4x. Here is the link to the Engine: https://github.com/richpeck/exception_handler/blob/master/lib/exception_handler/engine.rb#L19

The answer was to add the css and js files to the precompile array (which you're meant to not need with manifest.js). The trick for me was to add link_tree to the css / js files directly:

/*
  *= link_tree ../../images
*/

https://github.com/richpeck/exception_handler/blob/master/app/assets/stylesheets/styles/_base.css.erb

This way, the image files were included in the precompilation and from there, the extra dependencies were included too. I found out about link etc from the sprockets readme.

This works with Sprockets 3x and 4x in Heroku and our custom production environment.

shageman commented 7 years ago

Just leaving this here for the future: Forcing the existence of this file in app/assets/config is the first change since engines got introduced that makes it required for a Rails app to have an app folder at all. While that might be an unusual step, it is possible: I have been in the habit of putting all app code into engines from the start for a long time.

dreyks commented 7 years ago

yes! my usecase exactly. i do not have the app folder too

andyw8 commented 5 years ago

Just FYI @dreyks, this GitHub issue is referenced in the book Component Based Rails Applications (chapter 2).