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

Add docs about Webpack Encore. #120

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bellisk
Copy link

@bellisk bellisk commented Mar 12, 2019

No description provided.

Copy link
Contributor

@dbu dbu left a comment

Choose a reason for hiding this comment

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

cool! i wonder if we should also add this to the directories tree at the top of the page. however, as its optional, i think this better this way.

source/documentation/basic-project.md Outdated Show resolved Hide resolved
source/documentation/basic-project.md Outdated Show resolved Hide resolved
source/documentation/basic-project.md Show resolved Hide resolved
@bellisk bellisk force-pushed the hackday-introduce-webpack-encore branch 2 times, most recently from 79d41d6 to 3c03de0 Compare March 12, 2019 12:35
Copy link
Contributor

@dbu dbu left a comment

Choose a reason for hiding this comment

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

thanks for the improvements! looking over it, i noticed some more details - sorry for not having seen them in the first pass.

source/documentation/basic-project.md Outdated Show resolved Hide resolved
source/documentation/basic-project.md Outdated Show resolved Hide resolved
source/documentation/basic-project.md Outdated Show resolved Hide resolved

The app's main CSS file should be `source/assets/css/app.css`.
You can add more CSS files in the same directory if necessary and require them
in your JavaScript files to use them.
Copy link
Contributor

Choose a reason for hiding this comment

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

why javascript files? "... and require them in your app.css file to make them included."?

Copy link
Author

Choose a reason for hiding this comment

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

You can do it that way, but the preferred way is really to require them in your javascript files. (It's also possible to add CSS files to webpack.config.js by themselves, but again, not preferred. https://symfony.com/doc/current/frontend/encore/simple-example.html#compiling-only-a-css-file)

Copy link
Contributor

Choose a reason for hiding this comment

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

so the css is embedded in javascript? isn't that bad for page loading performance? or how should i understand "require"?

The [Blog Skeleton](https://github.com/sculpin/sculpin-blog-skeleton) project
uses Encore to manage assets. If you're setting up your project with Sculpin
directly, not using the skeleton, install Encore using the
[setup instructions](https://symfony.com/doc/current/frontend/encore/installation.html).
Copy link
Contributor

Choose a reason for hiding this comment

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

i'd add one more paragraph here saying that you need to install yarn and link to https://yarnpkg.com/en/docs/install for installation instructions

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure that's necessary, as the Encore instructions start off by telling you to install Node and Yarn, and link to the installation docs for both.

Copy link
Author

Choose a reason for hiding this comment

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

The sculpin-blog-skeleton README mentions using yarn to install the JS dependencies, but doesn't talk about installing yarn itself. I think if we are going to mention that anywhere, it should be in that README. @dbu maybe you could add that to sculpin/sculpin-blog-skeleton#61 ?

Copy link
Author

Choose a reason for hiding this comment

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

🤦‍♂️ Oh, you included that in sculpin/sculpin-blog-skeleton#61 already, sorry. I still don't think it's necessary to note it separately here.

Copy link
Contributor

Choose a reason for hiding this comment

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

k

@bellisk bellisk force-pushed the hackday-introduce-webpack-encore branch from 3c03de0 to 7083b71 Compare March 23, 2019 10:40
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.

None yet

3 participants