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

Rename javascript folder to frontend #1833

Closed
wants to merge 3 commits into from

Conversation

yvbeek
Copy link
Contributor

@yvbeek yvbeek commented Dec 14, 2018

With all the styles, images and other resources it might be better to call the folder frontend instead of javascript. This makes it easier to understand what the purpose of the folder is.

I did not rename the lib/install/javascript in the repository to prevent unnecessary conflicts.

Edit:
I have now also renamed the lib/install/javascript and test/test_app/app/javascript folders and a few more references to app/javascript that I missed. This should fix the failing tests.

@yvbeek yvbeek mentioned this pull request Dec 14, 2018
5 tasks
@gauravtiwari
Copy link
Member

Thanks @Zyphrax but please see: #2 and #692 and #1397

@yvbeek
Copy link
Contributor Author

yvbeek commented Dec 14, 2018

@gauravtiwari Wouldn't it be better for now to call the folder frontend instead of javascript?

It solves the discussion about javascript vs javascripts and it provides a better description of the files stored in the folder. A lot of people that are new to webpack(er) will be confused and might keep their images and styles in app/assets because they think they don't belong in the app/javascript folder.

@yvbeek
Copy link
Contributor Author

yvbeek commented Dec 14, 2018

See also this tutorial: https://evilmartians.com/chronicles/evil-front-part-1

rename it to frontend (or choose your own fancy name, but “frontend” makes most sense)

Or in this one: https://shakacode.gitbooks.io/react-on-rails/content/docs/tutorial.html

Edit your /config/webpacker.yml file. Change the default/source_path:
source_path: client

And this one: https://devato.com/new-rails-5-2-app-with-webpacker-and-react/

we're going to rename the javascript directory to frontend to keep things clear.
$ mv app/javascript app/frontend

@jakeNiemiec
Copy link
Member

We use the /client structure that @Zyphrax referenced, works well and aligns with industry conventions. However, since you can customize this in config/webpacker.yml I don't see the need to change this for everyone. Perhaps a README section on how to customize this?

@yvbeek
Copy link
Contributor Author

yvbeek commented Dec 14, 2018

@jakeNiemiec New users probably won't start by customizing config/webpacker.yml, that's why I thought a default that more accurately describes the contents of the folder could be useful.

As far as I can tell this change will only influence new users, it shouldn't break existing webpacker configurations, because updating the package won't change the source_path variable in config/webpacker.yml

With Rails 6 webpacker will probably replace the sprockets assets pipeline. It would be good to help people move away from app/assets and use webpacker for not only javascripts, but for resources like css/sass/coffeescript/images too.

@memiux
Copy link

memiux commented Dec 16, 2018

In my opinion, I think that for a Single Page Application, is a little weird naming it javascript.

@ameft
Copy link

ameft commented Dec 18, 2018

With Rails 6 webpacker will probably replace the sprockets assets pipeline. It would be good to help people move away from app/assets and use webpacker for not only javascripts, but for resources like css/sass/coffeescript/images too.

Problem is @Zyphrax that DHH is not of the same opinion rails/rails#33079 (comment) :

No, Sprockets is still to be used for SCSS (and other CSS processors) as
well as copying all other static assets.

@rmacklin
Copy link
Contributor

Here's an idea that I think could satisfy both types of users:

  1. Make the install task accept a --source-path argument so that you could call:
    webpacker:install --source-path=app/frontend
  2. Update the rails app generator such that If rails new appname --skip-sprockets is used to create a new rails app, the app generator will invoke webpacker:install --source-path=app/frontend (still defaulting to app/javascript if --skip-sprockets is not used).

The result would be that regular rails new still gives you both sprockets and webpacker using the app/assets and app/javascript directories (same as the current behavior), but if you explicitly opt out of sprockets using the existing --skip-sprockets flag in the generator, you only get webpacker (assuming you didn't also pass --skip-javascript) using the app/frontend directory.

@yvbeek
Copy link
Contributor Author

yvbeek commented Dec 18, 2018

@ameft If not Rails 6, then probably Rails 7. Sprockets isn't dead yet, but when the Rails community starts moving over to Webpack there would be little use to maintain two asset pipelines.

The pull request / discussion is about the (default) name of the app/javascript folder. In my opinion javascript is confusing, because the Webpack platform is about more then javascript. It would be a bit like calling the app folder ruby.

But lets not make it a big deal, it was just a suggestion 🙂

@ameft
Copy link

ameft commented Dec 18, 2018

@Zyphrax the name right now is javascript simply because the creator thinks to use Webpack(er) only for JS as a default in Rails, it's pretty clear. To change the name you have to change the scope of Webpack(er) inside of Rails. This doesn't seem the intention from the main maintainers.
Although I agree with you and others that it would be better to focus on a single assets management solution.
I like @rmacklin's idea though.

@yvbeek yvbeek closed this Dec 18, 2018
@justin808
Copy link
Contributor

@jakeNiemiec @gauravtiwari @dhh Given the prevalence of Webpack for all frontend (aka client) assets, should we revisit the default?

@dhh
Copy link
Member

dhh commented Feb 3, 2020

Nothing has changed on the fundamentals for me. We're happily using Webpack for JavaScript and the asset pipeline for styles and static assets in a brand new app. I find it to be a great combination.

Happy to entertain a patch that makes it easier to rename this for people who'd like to do that, if there are any blockers in that department, but no appetite for changing the defaults at the moment.

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 this pull request may close these issues.

8 participants