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

[4.3] Updating additional npm dependencies #38606

Closed
wants to merge 5 commits into from

Conversation

Hackwar
Copy link
Member

@Hackwar Hackwar commented Aug 26, 2022

After creating #38605, I also did update all other npm dependencies with a npm update. This update also includes the changes from #38605.

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.2-dev labels Aug 26, 2022
@Hackwar
Copy link
Member Author

Hackwar commented Aug 26, 2022

npm fails with an error in the CSS.

@brianteeman
Copy link
Contributor

and the error is?

@Hackwar
Copy link
Member Author

Hackwar commented Aug 26, 2022

Error: Undefined variable.

   ╷
20 │   @each $color, $value in $theme-colors-rgb {
   │                           ^^^^^^^^^^^^^^^^^
   ╵
  build/media_source/templates/administrator/atum/scss/_root.scss 20:27            @import
  build/media_source/templates/administrator/atum/scss/vendor/_bootstrap.scss 8:9  @import
  build/media_source/templates/administrator/atum/scss/template.scss 8:9           @import
  build/media_source/templates/administrator/atum/scss/template-rtl.scss 1:9       root stylesheet

npm ERR! code 1
npm ERR! path /********/src
npm ERR! command failed

@brianteeman
Copy link
Contributor

yeah sorry - I saw it after I posted and you beat me to it before I could say whoops

@brianteeman
Copy link
Contributor

Pretty sure I know what it is - just testing locally first.

@brianteeman
Copy link
Contributor

The problem is that npm update has updated to bootstrap 5.2 which definitely cannot be merged at this time.

in the package.json
change
"bootstrap": "^5.1.3",
to
"bootstrap": "~5.1.3",

and then rerun npm update

@laoneo laoneo changed the base branch from 4.2-dev to 4.3-dev August 26, 2022 11:00
@laoneo laoneo requested a review from rdeutz as a code owner August 26, 2022 11:00
@laoneo laoneo changed the title [4.2] Updating additional npm dependencies [4.3] Updating additional npm dependencies Aug 26, 2022
…ranch_4.2-update-dependencies2

# Conflicts:
#	package-lock.json
@Hackwar
Copy link
Member Author

Hackwar commented Aug 26, 2022

Fixed.

@brianteeman
Copy link
Contributor

Sorry this still fails - you can not blindly run npm update as there are several build scripts that run based on them.

In this specific case codemirror has been updated so the script that generated the codemirror plugin has to be run and will result in at least an updated codemirror.xml file.

Sadly despite their being a pull request for this specific update it was ignored and hence you have the problem here. #38343

In other words if you want to do an npm update and/or a composer update then you must also run the build scripts before you submit the pull request.

I've mentioned this a few times and that this should be automated but sadly as ever it fell on deaf ears.

From memory this effects jooa11y, codemirror, tinymce, skipto, accessibility and maybe some more

@Hackwar
Copy link
Member Author

Hackwar commented Aug 26, 2022

No, I don't know about these tasks and you never specifically pointed that information towards me. Now I know about this and I will write it on my long list of todos. I try to not be deaf to stuff like this. That is why I listened to Dimitris regarding Renovatebot or why I'm still working on joomla/framework.joomla.org#46, besides lots of other stuff. And no, I'm not happy that all of this is taking so long.

@brianteeman
Copy link
Contributor

Apologies if you were not one of the people I have mentioned this to. I definitely said it George, Harold and Roland and Dmitris already knew because he wrote the code.

@Quy Quy removed the PR-4.2-dev label Sep 8, 2022
@Hackwar
Copy link
Member Author

Hackwar commented Sep 22, 2022

I'm rather fixing renovatebot to get this right also for the future instead of merging this one. Closing.

@Hackwar Hackwar closed this Sep 22, 2022
@Hackwar Hackwar deleted the 4.2-update-dependencies2 branch September 22, 2022 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NPM Resource Changed This Pull Request can't be tested by Patchtester Unit/System Tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants