🏗Always build amp-loader and amp-auto-lightbox. #24137
Conversation
- It is easy to forget to build these extensions when running locally. Since these are loaded by the runtime, they should be built in order to get a more accurate represnetation of how AMP behaves when manually testing changes.
- Do not build all extensions when specified extensions are empty, since we always build some extensions (amp-laoder, amp-auto-lightbox). - Stop using the gulp entry point to build CSS from dep-check, instead do the same things (update pacakges, priint help message and call the compile CSS with option to build all.
|
Had to make some more changes, since there was an assumption that having no extensions to build meant to build all the extensions. I fixed dep-check which was relying on this assumption, but I may have missed others that also have the same problem. |
|
Looks like this actually also applies to the dist target. I think I need to change the approach here. |
You're right. There's another problem too. I too will play around with the code to see if I can spot a simple fix. |
There was a problem hiding this comment.
I'd like to push back against several of the changes you've made because I believe there's a cleaner way to do what you're trying to do.
options.compileAllwas added years ago, and is no longer necessary after recent refactors. (I've meant to remove it for a while.)options.compileOnlyCssis a sufficient signal forbuildExtensionsto compile all the CSS and no JS.- I think the way to do this is to roll back this PR to 454560e and then remove
options.compileAllfrombuildExtensions.
I'll try to come up with a working version that includes your changes until 454560e. SG?
Sounds good. |
It is easy to forget to build these extensions when running locally.
Since these are loaded by the runtime, they should be built in order to
get a more accurate representation of how AMP behaves when manually
testing changes.
@privatefor things that are not exported.