Sy 305/bootstrap extract #18752
Sy 305/bootstrap extract #18752
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a "Shop Bootstrap separation" feature that extracts Bootstrap into a dedicated webpack entry point, allowing users to disable Bootstrap entirely via the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
❗ Preview Environment deployment failed on BunnyshellSee: Environment Details | Pipeline Logs Check https://github.com/Sylius/Sylius/actions/runs/21396728660 for details. Available commands:
|
bd5fe47 to
35dd531
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@UPGRADE-2.2.md`:
- Around line 68-79: Update the upgrade note to explicitly instruct disabling
the Twig `bootstrap` hook when `includeBootstrap: false` is used in Webpack;
mention the exact hook identifiers `sylius_shop.base#stylesheets` and
`sylius_shop.base#javascripts` and the `bootstrap` hook name so users know to
set that hook's `enabled` to false (or remove the bootstrap template) to avoid
rendering missing entries when `includeBootstrap` is false in the
`SyliusShop.getBaseWebpackConfig` call.
| This allows disabling Bootstrap entirely if you want to use a different CSS framework. | ||
|
|
||
| **If you have overridden the `styles` hook** in `sylius_shop.base#stylesheets` and included Bootstrap there, it may now load twice. To fix this, either: | ||
| - Remove Bootstrap from your custom `styles` hook, or | ||
| - Disable the new `bootstrap` hook by setting `enabled: false` | ||
|
|
||
| **To disable Bootstrap in webpack**, pass `includeBootstrap: false` to the config: | ||
|
|
||
| ```js | ||
| const shopConfig = SyliusShop.getBaseWebpackConfig(path.resolve(__dirname), { | ||
| includeBootstrap: false | ||
| }); |
There was a problem hiding this comment.
Document the required hook disable when includeBootstrap=false.
As written, users may follow the webpack snippet but still render the bootstrap hook, which will try to load a missing entry and fail at runtime. Please add an explicit note to disable the bootstrap hook when includeBootstrap is false.
✏️ Suggested doc tweak
**To disable Bootstrap in webpack**, pass `includeBootstrap: false` to the config:
```js
const shopConfig = SyliusShop.getBaseWebpackConfig(path.resolve(__dirname), {
includeBootstrap: false
});+Also disable the bootstrap Twig hook (or remove the bootstrap template) to avoid rendering the missing entry:
+```yaml
+sylius_twig_hooks:
- hooks:
-
'sylius_shop.base#stylesheets': -
bootstrap: -
enabled: false -
'sylius_shop.base#javascripts': -
bootstrap: -
enabled: false
+```
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
In @UPGRADE-2.2.md around lines 68 - 79, Update the upgrade note to explicitly
instruct disabling the Twig bootstrap hook when includeBootstrap: false is
used in Webpack; mention the exact hook identifiers
sylius_shop.base#stylesheets and sylius_shop.base#javascripts and the
bootstrap hook name so users know to set that hook's enabled to false (or
remove the bootstrap template) to avoid rendering missing entries when
includeBootstrap is false in the SyliusShop.getBaseWebpackConfig call.
</details>
<!-- fingerprinting:phantom:poseidon:eagle -->
<!-- This is an auto-generated comment by CodeRabbit -->
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.