-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Bye bye gulp #12043
Bye bye gulp #12043
Conversation
Hi @kulczy! Didn't you forget the logo in the checkout? |
@jacquesbh good point, thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice nice nice!
@@ -1,5 +1,5 @@ | |||
<a class="item" href="{{ path('sylius_admin_dashboard') }}" style="padding: 13px 0;"> | |||
<div style="max-width: 90px; margin: 0 auto;"> | |||
<img src="{{ asset('assets/admin/img/admin-logo.svg') }}" class="ui fluid image"> | |||
<img src="{{ asset('build/admin/images/admin-logo.svg', 'admin') }}" class="ui fluid image"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes seem like an obvious candidate for UPGRADE file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we reduce those changes to be as minimal as possible? Leaving the asset path the same as before?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually this is not possible. This is how Encore works.
And keeping the same path will lead to some conflicts with other bundles.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lchrusciel @pamil Its possible..
.configureFilenames({
images: Encore.isProduction() ? 'img/[name].[hash:8].[ext]' : 'img/[name].[ext]',
})
See the complete code implementation below in the thread.
@@ -1 +1 @@ | |||
{% include '@SyliusUi/_javascripts.html.twig' with {'path': 'assets/admin/js/app.js'} %} | |||
{{ encore_entry_script_tags('admin-entry', null, 'admin') }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be documented as well
We have been using Webpack for our Sylius compilation for a long time, without changes in Sylius Twig admin templates. Even in production and without a problem. I am sending our configuration for inspiration. const Encore = require('@symfony/webpack-encore')
if (!Encore.isRuntimeEnvironmentConfigured()) {
Encore.configureRuntimeEnvironment(process.env.NODE_ENV || 'dev')
}
// Shop config
Encore.setOutputPath('public/assets/shop/')
.setPublicPath('/assets/shop')
.splitEntryChunks(Encore.isProduction())
.enableSingleRuntimeChunk()
.cleanupOutputBeforeBuild()
.enableBuildNotifications()
.enableSourceMaps(!Encore.isProduction())
.enableVersioning(Encore.isProduction())
.enableIntegrityHashes(Encore.isProduction())
.enableSassLoader()
.enableVueLoader(() => {}, { runtimeCompilerBuild: false })
.autoProvidejQuery()
.configureBabel(
(babelConfig) => {
babelConfig.plugins.push('@babel/plugin-proposal-class-properties')
},
{
includeNodeModules: ['@oxyshop'],
}
)
.configureBabelPresetEnv((config) => {
config.useBuiltIns = 'usage'
config.corejs = 3
})
// Fix issue: https://github.com/symfony/webpack-encore/issues/808
.configureUrlLoader({
images: {
limit: 0,
esModule: false,
},
})
.addEntry('shop-entry', './assets/shop/entry.js')
const shopConfig = Encore.getWebpackConfig()
shopConfig.name = 'shop'
Encore.reset()
// Admin config
Encore.setOutputPath('public/assets/admin/')
.setPublicPath('/assets/admin')
.splitEntryChunks(Encore.isProduction())
.enableSingleRuntimeChunk()
.cleanupOutputBeforeBuild()
.enableBuildNotifications()
.enableSourceMaps(!Encore.isProduction())
.enableVersioning(Encore.isProduction())
.enableIntegrityHashes(Encore.isProduction())
.enableSassLoader()
.autoProvidejQuery()
.configureBabelPresetEnv((config) => {
config.useBuiltIns = 'usage'
config.corejs = 3
})
.configureFilenames({
images: Encore.isProduction() ? 'img/[name].[hash:8].[ext]' : 'img/[name].[ext]',
})
// Fix CKEditor 4 integration into Webpack
.copyFiles([
{
from: './node_modules/ckeditor/',
to: 'ckeditor/[path][name].[ext]',
pattern: /\.(js|css)$/,
includeSubdirectories: false,
},
{ from: './node_modules/ckeditor/adapters', to: 'ckeditor/adapters/[path][name].[ext]' },
{ from: './node_modules/ckeditor/lang', to: 'ckeditor/lang/[path][name].[ext]' },
{ from: './node_modules/ckeditor/plugins', to: 'ckeditor/plugins/[path][name].[ext]' },
{ from: './node_modules/ckeditor/skins', to: 'ckeditor/skins/[path][name].[ext]' },
])
// Fix issue: https://github.com/symfony/webpack-encore/issues/808
.configureUrlLoader({
images: {
limit: 0,
esModule: false,
},
})
.addEntry('admin-entry', './assets/admin/entry.js')
const adminConfig = Encore.getWebpackConfig()
adminConfig.externals = {
...adminConfig.externals,
window: 'window',
document: 'document',
}
adminConfig.name = 'admin'
module.exports = {
shop: shopConfig,
admin: adminConfig,
} Only scripts and stylesheets twig files was changed: Links:
Scripts:
webpack_encore.yaml: webpack_encore:
builds:
shop: '%kernel.project_dir%/public/assets/shop'
admin: '%kernel.project_dir%/public/assets/admin'
output_path: false assets.yaml: framework:
assets:
json_manifest_path: '%kernel.project_dir%/public/assets/admin/manifest.json'
packages:
shop:
json_manifest_path: '%kernel.project_dir%/public/assets/shop/manifest.json' We use own implementation of Shop templates based on Bootstrap. But you can customize configureFilenames method for it. Cheers! 🚀 |
Hi, @misaon thanks for that. It looks good, but I think it's a bit hacky because, in |
And @pamil @lchrusciel when it comes to paths, I can leave current ones, but it can cause some problems during migration. The first that comes to mind is that webpack clear folders before every build, so if these folders contain any files that have been added manually or does not compile via webpack, they will be gone. I think that new folders will be a better idea, WDYT? |
@kulczy you're right this solution is a bit hacky. however, if people use sylius and do not want to change assets in all of their templates on shop front, this proposal is the easiest way to achieve this without work. You can reverse this logic and set shop as the default package, not administration. then the changes in administration would take effect after updating the sylius composer package. |
We might want to update to 1.0.0 https://github.com/symfony/webpack-encore/releases Watch out with the new version of webpack encore 1.0.0 the manifest.json is all wrong.
|
Now it's ok to use webpack encore 1.0.6 https://github.com/symfony/webpack-encore/releases/tag/v1.0.6 |
Why is this not moving forward? |
9b78ab7
to
e5281ca
Compare
@vvasiloi now we have to play with it, then implement it to the Standard. If after 2 yrs of working with it, you have any ideas about what could go wrong, let me know |
@kulczy I will look into it in weekend. Besides the Sylius-Standard, maybe also update the plugin skeleton and add support for themes out-of-the-box? |
I have webpack and encore in my project working for months as well. It makes loading script in Sylius much easier. This is what my import 'sylius/bundle/AdminBundle/Resources/private/entry';
import { startStimulusApp } from '@symfony/stimulus-bridge';
// Registers Stimulus controllers from controllers.json and in the controllers/ directory
export const app = startStimulusApp(require.context(
'@symfony/stimulus-bridge/lazy-controller-loader!./controllers',
true,
/\.(j|t)sx?$/
)); |
@pamil @lchrusciel I would love to see this in 1.10 ;) |
package.json
Outdated
"babel-preset-env": "^1.7.0", | ||
"babel-register": "^6.26.0", | ||
"dedent": "^0.7.0", | ||
"@symfony/webpack-encore": "^1.1.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upgrading to v2 would be nice 😃
Hello again xD After rebase, it looks like everything works, even with the new theme, but there are a few things to mention. I had to replace Semantic with Fomantic, due to a typo in Semantic css file 🤦♂️ - Webpack 5 will not compile it anymore. Of course, it's not so easy xD - Fomantic fixed it but this has not yet arrived in a stable release, so for now, I had to use beta version. Because the whole styles are changed, minor CSS bugs may occur. The second thing - upgrade instructions. As probably everybody managed the assets in their own way, I have no idea what to write in the Upgrade file to make it easier for people to update from gulp to webpack, so I need your help. If anyone has an idea, feel free to commit |
@kulczy I see some tests are failing due to
Idk if it was failing previously 😿 , I'll check it later and open next PR against your fork as previously. |
I think this is a nogo. See #10463 (comment) But as an alternative I suggest you to add a package that fixes it: // packages.json
{
// ...
"devDependencies": {
// ...
"@semantic-ui-react/css-patch": "^1.0.0",
},
"scripts": {
"postinstall": "semantic-ui-css-patch"
// ...
}
// ...
} This is suboptimal but responds to the nogo of the sylius team, we use it in production and it works just fine. (I prefer your solution) |
FYI, using the GitHub sources of Semantic-UI directly, the commit fixing the typo can be imported without any further patching needed: This package.json {
"dependencies": {
"@symfony/webpack-encore": "^2.1.0",
"semantic-ui-css": "github:Semantic-Org/Semantic-UI-CSS"
}
} compiles without issues for us. (See Semantic-Org/Semantic-UI-CSS/issues/81, the project seems to be pretty dead.) |
I have another idea, let's put this file into our repo. This is only one unmaintained CSS. By doing that, we could fix any errors regularly, and if a better option appears, we will go back to npm. WDYT? |
I think it's time to say goodbye to the Gulp. This is the next part of the transition started #10803
Assets can be compiled using the following commands:
yarn dev
yarn build
andyarn watch
I also encourage you to read the Webpack Encore documentation