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

Closes #622: Apply routine dev dependency updates. #623

Merged
merged 7 commits into from Jan 6, 2023
Merged

Conversation

mmunro-ltrr
Copy link
Member

@babel/core ^7.20.2 → ^7.20.5
@rollup/plugin-babel ^6.0.2 → ^6.0.3
@rollup/plugin-commonjs ^23.0.2 → ^23.0.3
eslint ^8.27.0 → ^8.28.0
find-unused-sass-variables ^4.0.4 → ^4.0.5
hugo-bin ^0.93.0 → ^0.95.0
linkinator 4.1.0 → 4.1.1
node-sass ^7.0.3 → ^8.0.0
postcss ^8.4.18 → ^8.4.19
postcss-cli ^10.0.0 → ^10.1.0
rollup ^2.79.1 → ^3.5.1
stylelint ^14.14.1 → ^14.15.0
stylelint-config-twbs-bootstrap ^6.0.0 → ^7.0.0
terser 5.15.1 → 5.16.1

Holding back on eslint-plugin-unicorn (#620) until the upstream twbs PR Build(deps-dev): Bump eslint-plugin-unicorn from 44.0.2 to 45.0.1 #37555 is resolved.

@mmunro-ltrr mmunro-ltrr added the dependencies Pull requests that update a dependency file label Dec 2, 2022
@mmunro-ltrr mmunro-ltrr requested a review from a team December 2, 2022 18:08
@mmunro-ltrr mmunro-ltrr self-assigned this Dec 2, 2022
@mmunro-ltrr
Copy link
Member Author

This now needs updated with the current version bumps, and there's now a minor change to the upstream Twitter Bootstrap .eslintrc.json file to help the eslint-plugin-unicorn update (now merged there).

joeparsons
joeparsons previously approved these changes Dec 20, 2022
@joeparsons joeparsons added the backport Backport to 2.x label Dec 20, 2022
@joeparsons
Copy link
Member

joeparsons commented Dec 20, 2022

I'm thinking we may want to hold back on bumping the major version for rollup until we're further along with being compatible with Bootstrap 5.x.

@trackleft noticed some strange [object Object] references showing up in the autogenerated banner section of the compiled JS files in this PR branch that I think happening because our rollup.config.js file is still more similar to the twbs v4-dev version of that file. The twbs main/5.x version of that file has some differences related to the banner script.

(e.g. see https://github.com/twbs/bootstrap/blob/v4-dev/build/rollup.config.js#L36 vs https://github.com/twbs/bootstrap/blob/main/build/rollup.config.js#L43)

@mmunro-ltrr
Copy link
Member Author

@joeparsons : I'll back out that change & re-roll the rest.

@mmunro-ltrr
Copy link
Member Author

Rather than downgrading rollup, the latest change applies the same fix to rollup.config.js that twbs made a few weeks ago. One other upstream change also comes in with the latest commit: twbs is again pinning the version of the terser package to a specific version because newer versions had increased the size of the files it generates. Linting checks were failing for a very obscure reason. There's a navbar example outside the main docs at
https://digital.arizona.edu/arizona-bootstrap/docs/2.0/examples/navbar/
The latest vnu-jar part of the linting checks has taken a dislike to this, objecting that the document specifies English as the language, but the content is in Catalan (it's really the Latin placeholder text). Changing the text to English kept it happy
https://review.digital.arizona.edu/arizona-bootstrap/issue/622/docs/2.0/examples/navbar/
But we might want to do something else with this generally overlooked demo content.

@trackleft
Copy link
Member

The Javascript appears to all still work, at least in my browser (Chrome Version 108.0.5359.124).

@@ -33,7 +33,7 @@ if (BUNDLE) {
module.exports = {
input: path.resolve(__dirname, '../js/src/index.js'),
output: {
banner,
banner: banner(),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
banner: banner(),
banner: banner(),
interop: 'amd',

Copy link
Member

@trackleft trackleft Dec 21, 2022

Choose a reason for hiding this comment

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

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'm not seeing this as an option in
https://rollupjs.org/guide/en/#outputinterop
and it does seem to break the build locally.

package.json Outdated
"@rollup/plugin-babel": "^6.0.2",
"@rollup/plugin-commonjs": "^23.0.2",
"@rollup/plugin-babel": "^6.0.3",
"@rollup/plugin-commonjs": "^24.0.0",
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this plugin is actually used in our rollup config. Looks like twbs/bootstrap only uses it in tests (as far as I can tell).

Suggested change
"@rollup/plugin-commonjs": "^24.0.0",

Copy link
Member

Choose a reason for hiding this comment

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

FWIW, I think it was me who originally added the rollup plugins (way back in #108) and IIRC, I just included all the rollup packages included in twbs/bootstrap's package.json at the time without doing much detailed investigation about where they were used... (my bad).

Copy link
Member Author

Choose a reason for hiding this comment

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

Deleting that plugin doesn't modify dist/js (or dist/css) in any way. Looks good to omit.

@mmunro-ltrr
Copy link
Member Author

Testing the suggestions locally, before pushing any changes....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Backport to 2.x dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants