-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
feature/encore-webpack #5461
feature/encore-webpack #5461
Conversation
@OskarStark it never worked with jQuery 3 see https://github.com/sonata-project/SonataAdminBundle/blob/3.x/src/Resources/public/vendor/jquery/dist/jquery.js The DateTimePicker lib just works with jQuery 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.
Please add new line
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 is generated
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.
Argh this should not happen, do you know where we need to head to provide a PR?
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.
cc @weaverryan
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.
friendly ping @weaverryan
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.
@OskarStark i figured out:
manifest.json
is generated: https://github.com/symfony/webpack-encore/blob/master/lib/plugins/entry-files-manifest.js#L80
But for entrypoints.json
i didn't find the place where that happens
Yes we should do this IMO |
Could you please rebase your PR and fix merge conflicts? |
@OskarStark i think a configuration option is not good for this, because this config is related to WebpackEncore. Maybe i update the FlexRecipe of SonataAdminBundle? What would you say? |
Should we include the dist folder (https://github.com/silasjoisten/SonataAdminBundle/tree/feature/encore-webpack/src/Resources/public/dist)? This disallows using the latest (secure) version of a frontend library. |
Could you please rebase your PR and fix merge conflicts? |
The other opportunity would be pushing to npm.js but this is not that as good because this "lib" wont work without the php stuff. I think its good to commit the dist folder and update manually from time to time. Other thing is that many frontend libs does not do sematic versioning so can be problematic to update them. |
f021cd4
to
02a7752
Compare
Could you please rebase your PR and fix merge conflicts? |
02a7752
to
a365908
Compare
Can you give a short status report @silasjoisten ? Is this stable enough, so we can review and merge this? |
src/DependencyInjection/Compiler/WebpackEntriesCompilerPass.php
Outdated
Show resolved
Hide resolved
tests/DependencyInjection/Compiler/WebpackEntriesCompilerPassTest.php
Outdated
Show resolved
Hide resolved
…est.php Co-Authored-By: Christian Gripp <[email protected]>
Co-Authored-By: Christian Gripp <[email protected]>
Co-Authored-By: Christian Gripp <[email protected]>
Can you fix CS please? |
src/DependencyInjection/Compiler/WebpackEntriesCompilerPass.php
Outdated
Show resolved
Hide resolved
Could you please rebase your PR and fix merge conflicts? |
The Checklist is completed? Is this RTM @silasjoisten ? |
Could you please rebase your PR and fix merge conflicts? |
@silasjoisten Thanks for your work 👍 |
@silasjoisten I'm working on your webpack. I will fix commits structure. . Dont use merge (branch) to update data. It blocks previus commits and make history ugly. Also use |
Hi @silasjoisten ! Is something missing in this PR or is it RTM ?
Do you know what is missing to finish this PR @wbloszyk ? |
There is still some work to be done. I also work with assets while sonata-project/dev-kit#697. After drop core i can finish this PR. Also will be nice to have upgraded sandbox to add encore to all sonata bundles (I have done AdminLTE and Bootstrap 4 branch). Can you check: |
Can we help with anything to move this PR forward? |
@nieuwenhuisen This PR is not contributing for some time. I worked on it too. It is now move to sandbox (webpack branch). Here is issue for it: BTW |
This PR should be done in https://github.com/sonata-project/sandbox/tree/webpack as proof of concept. Then we can easy move it to this repository. Thank you for you current work @silasjoisten. It is awesone example. If you want help and work on it feel free to reopen this PR. |
Subject
Closes #5454
webpack_encore.builds
and adding them to templateassets
undersonata_admin
Review
src/Resources/public/dist
sonata_admin.yaml
orwebpack_encore.yaml
There is a compiler pass which is reading the
webpack_encore.builds
config and adding them to twig globals. In template there will automtically added.Reference
Related PR