-
-
Notifications
You must be signed in to change notification settings - Fork 151
draft of BApp #2732
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
base: main
Are you sure you want to change the base?
draft of BApp #2732
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@xvaara, I really like this for the core setup. It's what I would use in my app, given a choice between this and the current plugin. The main question I have is about tree shaking. As @VividLemon mentioned previously, there is some segment of our user base that is quite concerned about the deployed size of BSVN. If we eventually deprecate the plugins and only allow this method of installation, do we effectively disable tree shaking? |
I think the best solution to this is to effectively give people the tools to build their own "bapp" and document how to do it well |
teleportTo: undefined, | ||
defaults: undefined, | ||
mergeDefaults: false, | ||
deepMerge: false, |
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.
Maybe mergeDefaults is boolean | function
to where the function is the algorithm to use for merging?
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.
I generally dislike props that just effect the behavior of other props. Begs the question why it's not just an object
@dwgray BApp or the orchestrators imports nothing complex, just keys and conditionalTeleport, so if the user doesn't import them or the composables there shouldn't be any extra components included. |
@xvaara I've been thinking about this some more. If this doesn't significantly affect tree shaking, I am still a bit torn. I'd very much prefer it if we land on one solution or the other, rather than maintaining both. @VividLemon 's suggestion of "give people the tools to build their own 'bapp' and document how to do it well" sounds appealing as a solution to only maintaining one implementation, but that really ends up being what I consider a Advantages of BApp:
Advantages of Plugin:
Overall, that does land me on seeing if we can make BApp work, again assuming it doesn't affect tree shaking. I started digging into the mechanics of tree-shaking to convince myself that we would be safe, and quickly realized that this is one of those problems where I wouldn't be able to truly convince myself of the results without experimental evidence. I think it's probably good for us to be able to regression test tree shaking overall, since some part of our user base really cares about this (and others may care more if they dug into it). My basic idea around this is to have a small project that we can build and establish baseline sizes to confirm that we've not changed the pay-to-play nature of the tree shaking. We should then be able to just change the target bsvn installation in the package.json file and rebuild to see how sizes are affected. Here's the stats for a recent build without the BApp changes: Simple Vue.js/Vite app using create vue@latest
Add bootstrap and BSVN
Add a reference to BLink
Add a reference to BAvatar
Overall, these seem like reasonable results and show that adding references to new components increases the size of the packet (presumably more than the reference). I also did a bit of digging into the generated files to prove to myself that there weren't references to The place where I stalled out is that I wasn't able to change the reference to BSVN to @xvaara's BApp branch. I think that's because it doesn't build? But it may be because I don't have the syntax right. Here's what I tried:
I've pushed the project up to https://github.com/[dwgray/bsvn-tree-shake](https://github.com/dwgray/bsvn-tree-shake) - let me know if you think this is a reasonable way to approach confirming our tree-shaking (and that we don't degrade it with features like this) @xvaara and @VividLemon and if I should add other features to test (I'm thinking P.S. Looks like I accidentally wrote over your comment rather than replying to it - I think I managed to restore the original comment, but that's why it's tagged as edited by me... |
BApp Component
The
BApp
component is the new recommended way to configure bootstrap-vue-next. It replaces the plugin-based approach and provides better defaults management and orchestrator integration.Features
Basic Usage
Configuration
Setting Component Defaults
Advanced Defaults Configuration
Orchestrator Configuration
Disabling Orchestrators
RTL Configuration
Migration from Plugins
Before (Plugin-based)
After (BApp-based)
Props
defaults
Partial<BvnComponentProps>
undefined
mergeDefaults
boolean
false
deepMerge
boolean
false
teleportTo
TeleportProps['to']
undefined
noModals
boolean
false
noToasts
boolean
false
noPopovers
boolean
false
noOrchestrator
boolean
false
appendToast
boolean
false
inhert
boolean
false
rtl
boolean | RTLConfig
false
RTL Configuration Object
Composables
When using
BApp
, the following composables work without requiring plugin installation:useToastController()
- Create and manage toasts programmaticallyuseModalController()
- Create and manage modals programmaticallyusePopoverController()
- Create and manage popovers programmaticallyInternal Features
The
BApp
component automatically provides several internal services:Default Merging Behavior
The component supports three modes for handling defaults:
defaults
prop completely replaces any existing defaultsmergeDefaults: true
): Shallow merge with existing defaultsmergeDefaults: true, deepMerge: true
): Deep merge nested objectsTeleporting
By default, orchestrators render inline. Use
teleportTo
to render them elsewhere:Backward Compatibility
The
BApp
component is fully backward compatible with existing plugin-based setups. You can gradually migrate by:BApp
to your root componentBApp
propsThe plugins will show deprecation warnings but continue to work until removed in a future version.
Notes
BApp
or orchestrator should be active at a time per applicationteleportTo
is specifiedPR checklist
What kind of change does this PR introduce? (check at least one)
fix(...)
feat(...)
fix(...)
docs(...)
The PR fulfills these requirements:
CHANGELOG
is generated from these messages, and determines the next version type. Pull requests that do not follow conventional commits or do not have an override will be denied