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

Better modularization of our plugins #22888

Closed
wants to merge 9 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions .babelrc
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,9 @@
[
"es2015",
{
"loose": true,
"modules": false
}
]
],
"plugins": [
"transform-es2015-modules-strip"
]
"plugins": ["external-helpers"]
}
94 changes: 94 additions & 0 deletions build/build-plugins.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
const path = require('path')
const rollup = require('rollup')
const babel = require('rollup-plugin-babel')

const files = {
Util: path.resolve(__dirname, '../js/src/util.js'),
Alert: path.resolve(__dirname, '../js/src/alert.js'),
Button: path.resolve(__dirname, '../js/src/button.js'),
Carousel: path.resolve(__dirname, '../js/src/carousel.js'),
Collapse: path.resolve(__dirname, '../js/src/collapse.js'),
Dropdown: path.resolve(__dirname, '../js/src/dropdown.js'),
Modal: path.resolve(__dirname, '../js/src/modal.js'),
Popover: path.resolve(__dirname, '../js/src/popover.js'),
Scrollspy: path.resolve(__dirname, '../js/src/scrollspy.js'),
Tab: path.resolve(__dirname, '../js/src/tab.js'),
Tooltip: path.resolve(__dirname, '../js/src/tooltip.js'),
Index: path.resolve(__dirname, '../js/src/index.js')
}

const pathDest = path.resolve(__dirname, '../js/dist/')
const pathDestESM = path.resolve(__dirname, '../js/dist/esm/')
const relativePathSrc = 'js/src/'
const relativePathDist = 'js/dist/'
const relativePathDistESM = 'js/dist/esm/'
const externalIndex = []

for (const plugin in files) {
if (!Object.prototype.hasOwnProperty.call(files, plugin)) {
continue
}
const file = `${plugin.toLowerCase()}.js`
var externalArray = ['jquery', 'popper.js']

if (files[plugin] !== files.Util) {
externalArray.push(files.Util)
}

if (files[plugin] === files.Popover) {
externalArray.push(files.Tooltip)
}

// Do not export every plugins for the Index
if (files[plugin] !== files.Index) {
externalIndex.push(files[plugin])
}
else {
externalArray = externalIndex
}

rollup.rollup({
entry: files[plugin],
external: externalArray,
plugins: [
babel({
exclude: 'node_modules/**' // only transpile our source code
})
],
onwarn: function (warning) {
// Avoid warning for Tooltip and Util imports
if (warning.code === 'MISSING_GLOBAL_NAME') {
return
}
}
})
.then(function (bundle) {
const conf = {
dest: `${pathDest}/${file}`,
format: 'umd',
moduleName: plugin === 'Index' ? 'bootstrap' : plugin,
sourceMap: true,
globals: {
jquery: '$',
'popper.js': 'Popper'
}
}
// Write plugin in UMD
bundle.write(conf)
.then(function () {
console.log(`${relativePathSrc}${file} -> ${relativePathDist}${file}`)

conf.format = 'es'
conf.moduleName = null
conf.dest = `${pathDestESM}/${file}`
conf.globals = null

// Write plugin in ESM
bundle.write(conf)
.then(function () {
console.log(`${relativePathSrc}${file} -> ${relativePathDistESM}${file}`)
})
})
})
.catch(console.error) // log errors
}
43 changes: 43 additions & 0 deletions build/rollup.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
const path = require('path')
const fs = require('fs')
const babel = require('rollup-plugin-babel')
const resolve = require('rollup-plugin-node-resolve')
const BUNDLE = process.env.BUNDLE === 'true'
const year = new Date().getFullYear()

const dataPkg = fs.readFileSync(path.resolve(__dirname, '../package.json'))
const pkg = JSON.parse(dataPkg)

var fileDest = 'bootstrap.js'
var external = ['jquery', 'popper.js']
const plugins = [
babel({
exclude: 'node_modules/**' // only transpile our source code
})
]

if (BUNDLE) {
fileDest = 'bootstrap.bundle.js'
// remove last entry in external array ton bundle Popper
external.pop()
plugins.push(resolve())
}

module.exports = {
entry: path.resolve(__dirname, '../js/src/index.js'),
format: 'umd',
moduleName: 'bootstrap',
dest: path.resolve(__dirname, `../dist/js/${fileDest}`),
external: external,
globals: {
jquery: '$',
'popper.js': 'Popper'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These dependencies should be defined as external, but not global. Bootstrap should actively try to import them (mostly to fix #23381)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if I remove globals I have those warnings :
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's because you don't have any import $ from 'jquery' and import Popper from 'popper.js' in the code?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's wrong in this PR we import Popper.js and jQuery (for example see : https://github.com/twbs/bootstrap/blob/webpack/js/src/tooltip.js)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, probably you still need those for the UMD and IIFE bundles, but you must make sure no never reference to them in the source, but always require them with import

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If someone want to use Bootstrap in a UMD module a rollupjs config like this would work perfectly without : window.Popper = Popper :

image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this we can do require(['jquery', 'popper.js', 'bootstrap'], ($, Popper, Bootstrap) => {})?

Copy link
Member Author

@Johann-S Johann-S Aug 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently I have this issue :
⚠️ 'jquery' is imported by ..\bootstrap\js\dist\esm\scrollspy.js, but could not be resolved – treating it as an external dependency

So I'm not sure in an UMD build everything works too...

EDIT :
My ESM files import jQuery and that's why I have this issue 😩

RE EDIT :
jQuery is difficult to import... For now I cannot make it work without something like this :
<script src="https://code.jquery.com/jquery-3.2.1.slim.min.js"></script>
But for Popper.js it works perfectly

Do you have any ideas @FezVrasta ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope sorry, It's been a while since last time I used jQuery :/

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've got to be sure but it seems that's because jQuery do not have an ES6 release 😟

},
plugins: plugins,
banner: `/*!
* Bootstrap v${pkg.version} (${pkg.homepage})
* Copyright 2011-${year} ${pkg.author}
* Licensed under MIT (https://github.com/twbs/bootstrap/blob/master/LICENSE)
*/
`
}
41 changes: 0 additions & 41 deletions build/stamp.js

This file was deleted.

Loading