Skip to content
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

Update docs example on how to optimize JavaScript bundle #31111

Merged
merged 3 commits into from
Oct 20, 2020
Merged

Update docs example on how to optimize JavaScript bundle #31111

merged 3 commits into from
Oct 20, 2020

Conversation

pascalpepe
Copy link
Contributor

@pascalpepe pascalpepe commented Jun 19, 2020

The lean JavaScript example in v5 docs has not been updated from v4.

This PR suggests the two following changes in the Lean JavaScript code snippet:

  • Remove the import of js/dist/util.js, which does not exist anymore.
  • Change the comment about Popper.js to also mention dropdowns instead of just tooltips and popovers.

https://deploy-preview-31111--twbs-bootstrap.netlify.app/docs/5.0/customize/optimize/

@XhmikosR
Copy link
Member

Yeah, I think this should be cleaned up. See #31111

@pascalpepe
Copy link
Contributor Author

pascalpepe commented Jun 20, 2020

Alright, I just created this repo containing minimal working examples with Parcel, Rollup and webpack.

In all three examples, the bundle contains only the collapse and dropdown plugins (plus Popper.js, required for dropdowns). There's also a template with a navbar that uses both plugins.

Regardless of the bundler, imports are written like this:

import 'bootstrap/js/dist/collapse';
import 'bootstrap/js/dist/dropdown';

And actually there is no need to explicitly import Popper.js in the entry file because all bundlers will automatically include it if needed. It simply needs to be installed with npm in the same way as Bootstrap.

If this sounds ok for you, I can rebase and update this PR with cleaned imports and the correct comment about Popper.

@pascalpepe pascalpepe marked this pull request as draft June 22, 2020 14:31
@mdo
Copy link
Member

mdo commented Aug 3, 2020

@pascalpepe Holler if you need any more feedback here before opening up for reviews :).

@Johann-S
Copy link
Member

Johann-S commented Aug 6, 2020

LGTM @pascalpepe 👍

@pascalpepe
Copy link
Contributor Author

Thanks for reviews ! Here's the updated PR:

  • The .js extension has been removed from all imports as it is not needed.
  • To mirror the section about SCSS customization that's just above this one, I updated the JS example like this:
    • The example now lists all available JS components.
    • Only the modal component is not commented out (to mirror the comments above: "If you’re not using a component, comment it out or delete it entirely").
    • The comment about Popper.js was previously in the code snippet. I moved it at the end of section.

Ready for review :)

@pascalpepe pascalpepe marked this pull request as ready for review August 6, 2020 09:01
@pascalpepe pascalpepe requested a review from Johann-S August 6, 2020 18:46
mdo
mdo previously approved these changes Aug 6, 2020
@Johann-S
Copy link
Member

Johann-S commented Aug 7, 2020

Btw I think we should mention we use export default for those files, which mean if you want to create a new modal you have to do:

import Modal from 'bootstrap/js/dist/modal'

const modal = new Modal(document.getElementById('myModal'))

@mdo
Copy link
Member

mdo commented Sep 15, 2020

Any interest in the last comment @pascalpepe?

@XhmikosR
Copy link
Member

@Johann-S can you add the changes you mentioned above?

@Johann-S
Copy link
Member

Johann-S commented Oct 1, 2020

@XhmikosR I'm unable to add a JS example in a callout, do you know how to do that?

@XhmikosR
Copy link
Member

XhmikosR commented Oct 2, 2020

For some reason, the Netlify preview is broken; it throws a ReferenceError:

 Uncaught ReferenceError: ClipboardJS is not defined
    <anonymous> https://deploy-preview-31111--twbs-bootstrap.netlify.app/docs/5.0/assets/js/docs.min.js:38
    <anonymous> https://deploy-preview-31111--twbs-bootstrap.netlify.app/docs/5.0/assets/js/docs.min.js:52

I'll put this on hold, until I figure out why this happens.

@XhmikosR XhmikosR dismissed mdo’s stale review October 5, 2020 18:08

Breaks docs JS

@XhmikosR
Copy link
Member

XhmikosR commented Oct 5, 2020

Let's wait until we proceed with #31806 and #31802 because docs JS is broken with the current patch as is.

Johann-S and others added 2 commits October 20, 2020 11:00
For some weird reason, using "Exports" as the callout header leads to TypeError coming from clipboard.js
@XhmikosR
Copy link
Member

So, it seems the error comes from ClipboardJS due to the "Exports" heading. Should probably be reported upstream, but in the meantime I renamed the heading to "Default Exports".

If someone could report it, that would help.

@XhmikosR XhmikosR merged commit 4fca7dd into twbs:main Oct 20, 2020
@XhmikosR XhmikosR removed the on-hold label Oct 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants