Skip to content

[Webpack Encore] Change default assets directory structure #789

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

Merged
1 commit merged into from
Sep 30, 2020

Conversation

tgalopin
Copy link
Contributor

@tgalopin tgalopin commented Jun 18, 2020

Q A
License MIT
Doc issue/PR symfony/symfony-docs#14287

I propose to update the default assets directory structure present in the Webpack Encore recipe.

The reasoning behind this is the following:

  1. Nowadays, most front-end applications evolve around Javascript: by moving app.js to the root of the directory, we make it more obvious that it is the main entrypoint of the assets directory.
  2. When using React or SCSS, the default directories names are misleading: the js directory won't contains js files but jsx files, the css directory won't contain css files but scss files. By renaming css to styles, we clarify the aim instead of the format.
  3. By putting app.js at the root directory, we incentivize people to create their other endpoints on the same level: the root directory becomes an easy way to find all the entrypoints of an application.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request passes validation.

@javiereguiluz
Copy link
Member

In addition to app.js being at assets/ ... do you propose to store all JS in that directory as well ... or would they be kept at assets/js/.

Another question: you have experience in JS world ... do they use stylesheets/ dir to store CSS, SCSS, etc.? Do they use other practices for those files? Thanks!

@tgalopin
Copy link
Contributor Author

In addition to app.js being at assets/ ... do you propose to store all JS in that directory as well ... or would they be kept at assets/js/.

Yes, this is my proposal indeed: nowadays, in most front-end applications the Javascript is the most important bit of code running on the client. Having JS entrypoints at the root directory and then letting people have their imported scripts inside directories under assets reduce the length of paths, avoid relying on a format and increase discoverability.

Another question: you have experience in JS world ... do they use stylesheets/ dir to store CSS, SCSS, etc.? Do they use other practices for those files?

That's a good point. I had a quick look:

  • create-react-app creates CSS files at the same level as the JS files, because each CSS file is associated to its JS React component: src/App.js is the component, src/App.css is its styling. I don't think this applies in Webpack Encore as we usually have a single core CSS file and a few others sometimes, but not one per entrypoint/component.

  • Vue apps usually include their CSS inside their components, using a <style> tag that is parsed by Vue to build a global CSS file with namespaced selectors. I don't think it applies either.

  • Angular calls it styles: we could use this naming, it may be better suited (especially because in Encore, it's called a "style entry" => styles makes sense)

I used sylesheet as a reference to the <link> tag rel attribute, but we can definitely change it to styles if you prefer :) .

@tgalopin tgalopin force-pushed the new-encore-structure branch from d1020b5 to c8fd966 Compare September 25, 2020 13:59
@tgalopin tgalopin changed the title [Webpack Encore] WIP: Change default assets directory structure [Webpack Encore] Change default assets directory structure Sep 25, 2020
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request passes validation.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request passes validation.

@javiereguiluz
Copy link
Member

Let's ping some people who know Webpack Encore very well in case they find some problem with this: @weaverryan @Lyrkan @Kocal @stof Thanks!

@Kocal
Copy link
Member

Kocal commented Sep 25, 2020

Hi, and thanks for your proposal!

This seems very legit to me.

For my own case, on my projects and company projects, I always use a structure like this:

  • assets/app/index.js, for common stuff (utils functions, generic Vue components, ...)
  • assets/app/styles/... or assets/styles for common styles
  • assets/pages/<...>/index.js, for each pages of my application (= one webpack entry per page)

We also use <style> in our Vue components.

I don't think I ever used the actual js/css structure.

A big +1 for me!

@weaverryan
Copy link
Member

@tgalopin would you be able to make a docs PR? This will break some paths - we need to get those updated before merging this. But other than that, I think it's a great idea!

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request passes validation.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request passes validation.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request passes validation.

@tgalopin
Copy link
Contributor Author

I updated the PR and added documentation. This should be ready :) .

@ghost ghost merged commit bdf0da8 into symfony:master Sep 30, 2020
weaverryan added a commit to symfony/symfony-docs that referenced this pull request Sep 30, 2020
…ture (tgalopin)

This PR was submitted for the master branch but it was merged into the 5.1 branch instead.

Discussion
----------

[Webpack Encore] Change default assets directory structure

Adapt the documentation for symfony/recipes#789

Commits
-------

e5aa761 [Webpack Encore] Change default assets directory structure
@tgalopin tgalopin deleted the new-encore-structure branch September 30, 2020 13:35
This pull request was closed.
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.

6 participants