-
-
Notifications
You must be signed in to change notification settings - Fork 169
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 support for Magepack #138
Add support for Magepack #138
Conversation
Hi, You can see that the path is the local project path. |
…the appdata volume
…ainer image by overriding the cmd
@gfilice Seems like there was an error in where the compose config expected the dockerfile to be on this PR. I haven't really tested any of this, as I haven't had the chance, but dropping a quick note here as I've just updated the PR her per the following:
I verified that Won't be required to merge this PR, but I'd love to see a short docs page added that covers it's use similar to the docs added for MFTF. |
Thank you @davidalger. Just updated my Warden to 0.5.1 and set the magepack integration using your commits. Now everything works fine. |
Just ran through this real quick on 2.3.5-p1 and I have to say, it works flawlessly with the provided instructions. The below is example output from running the magepack commands, followed by the in-browser results on a PDP:
|
@vbuck Thanks for this contribution to Warden! With it, we've been able to deploy Magepack to an M2 site, and are working on another. We're running into one issue with Magepack that we're trying to troubleshoot, and it will require editing the
Do you have any tips to be able to do this? |
@erikhansen This message indicates that This will execute the magepack binary, and output help text:
The image build also contains wrappers for the two sub-commands allowing one to execute them like this:
or
|
@davidalger Thanks. The Btw, this is what we're working on implementing, thus why we need to be able to edit the |
magepack generate command is not working with another domain that setup in warden docker Ex. I have domain abc.com and bcd.com magepack generate is work with abc.com but not works with bcd.com |
Summary
This is a proposal and PoC to support Magepack for advanced JS bundling:
It works by creating an optional container from which to execute Magepack asset bundling. Editing
.env
to addWARDEN_MAGEPACK=1
will enable it duringwarden env up
operations.How to Use
If you follow the instructions for Magepack, you'll see there are a series of configuration changes to make for Magento, including the installation of a companion module. I'll post my version of those steps, which picks up where Warden ends (after environment initialization):
Drop into the shell:
warden shell
. Then run:Exit the shell, then run the following:
Replacing the URLs with that of your own environment. In my example, I initialized an M2 project at
app.warden-m2.test
and installed sample data.Magepack will generate the config needed and drop it into
/var/www/html/magepack.config.js
. Note that this generates on a shared mount with yourphp-fpm
container. This is by design, as the next step for Magepack will generate and place your bundles into the static content area.Finally, run:
When finished, you should be able to navigate to a page and observe the bundles at work; ie:
And here is the corresponding Lighthouse score (before/after):
Before Magepack:
After Magepack:
Additional Notes
You may notice I'm referencing a forked version of the Magepack repo. This is necessary because there are compatibility issues with various Linux installations when trying to setup Puppeteer for headless Chrome operation. My fork disables sandboxing and ignores TLS certificate errors. I believe this is a safe change to make, as Magepack is a build tool and generally should not be subject to exploits from external forces (read: risk is low).Update: Sandboxing has already been updated in Magepack, bound for a 2.2.1 release, and I've opened an issue for ignoring TLS errors. Once integrated we can refer to the official source.As of my commit bef0ef7, I am now using the official latest package @^2.3, so the above notes no longer apply.
Lastly, I'll acknowledge this is a feature intended for build testing, not necessarily daily development. So I'd like to hear your thoughts on its fit as an embedded Warden feature.