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

build: batch update javascript dependencies #839

Merged

Conversation

afuetterer
Copy link
Member

@afuetterer afuetterer commented Nov 23, 2023

Description

Next try.

This PR proposes the following changes:

  • update the Javascript dependencies in multiple grouped commits
  • alter the dependabot config slightly (move copy-webpack-plugin to the more fitting webpack update group
  • update the used versions of eslint and react plugin in pre-commit config
  • take a screenshot during frontend testing

Types of Changes

  • Build related changes

Checklist

  • I have read the contributor guide.
  • My code follows the code style of this project.
  • All new and existing tests passed.

Tasks

  • pin react-bootstrap
  • ignore updates for react-bootstrap

@afuetterer

This comment was marked as resolved.

@MyPyDavid
Copy link
Member

MyPyDavid commented Nov 24, 2023

thank you, Ive installed the wheel.

Looks like there is a bug with the icons?

Catalogs

image

@MyPyDavid
Copy link
Member

MyPyDavid commented Nov 24, 2023

and in the edit form:

Edit catalog

image

@MyPyDavid
Copy link
Member

Is this a bug in my setup with missing static files or from this new build? Do you have an idea @jochenklar?

@afuetterer
Copy link
Member Author

Can you try with a wheel from another PR, that did not touch the frontend dependencies?
Maybe this one: #833

Does 833 have icons?

@afuetterer
Copy link
Member Author

But the frontend does work in principle? You can click around, the buttons are working? I mean the frontend tests test exactly that.

It is "just" a matter of the icons, yes?

@MyPyDavid
Copy link
Member

yes the front-end was working fine and everthing is clickable.
Now with the #833 it looks normal

@afuetterer
Copy link
Member Author

Alright.
I did not build the frontend. There are a couple of changes in this PR. Maybe its a problem with the webpack bundle.

@jochenklar: Which library supplies the icons?

@jochenklar
Copy link
Member

I am not 100% which items you mean, but we use font awesome 4 icons. These are not bundled by webpack, yet but are included here: https://github.com/rdmorganiser/rdmo/blob/master/rdmo/core/templates/core/base.html#L9 (old school).

@afuetterer
Copy link
Member Author

image

I mean the missing icons @MyPyDavid mentioned.

Should font awesome be downloaded before building the wheel?

The build wheel does the steps, documented here:
https://rdmo.readthedocs.io/en/latest/development/releases.html

That would not make sense, if they were present in 833.

Did you run collectstatic @MyPyDavid?

@jochenklar
Copy link
Member

Maybe @MyPyDavid forgot download_vendor_files?

@afuetterer
Copy link
Member Author

download_vendor_files is supposed to called after installing rdmo, right? So these vendor files should not be in the wheel, correct?

@MyPyDavid
Copy link
Member

MyPyDavid commented Nov 24, 2023

I re-did download_vendor_files

24 Failed to decode downloaded font: <URL>
24 OTS parsing error: invalid sfntVersion: 1702391919

@afuetterer

This comment was marked as resolved.

@afuetterer afuetterer added javascript Pull requests that update Javascript code dependencies Pull requests that update a dependency file labels Nov 27, 2023
afuetterer and others added 2 commits November 29, 2023 06:49
Bumps the babel group with 5 updates:

| Package | From | To |
| --- | --- | --- |
| [@babel/cli](https://github.com/babel/babel/tree/HEAD/packages/babel-cli) | `7.17.10` | `7.23.4` |
| [@babel/core](https://github.com/babel/babel/tree/HEAD/packages/babel-core) | `7.17.10` | `7.23.3` |
| [@babel/preset-env](https://github.com/babel/babel/tree/HEAD/packages/babel-preset-env) | `7.17.10` | `7.23.3` |
| [@babel/preset-react](https://github.com/babel/babel/tree/HEAD/packages/babel-preset-react) | `7.16.7` | `7.23.3` |
| [babel-loader](https://github.com/babel/babel-loader) | `8.2.5` | `9.1.3` |

Updates `@babel/cli` from 7.17.10 to 7.23.4
- [Release notes](https://github.com/babel/babel/releases)
- [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md)
- [Commits](https://github.com/babel/babel/commits/v7.23.4/packages/babel-cli)

Updates `@babel/core` from 7.17.10 to 7.23.3
- [Release notes](https://github.com/babel/babel/releases)
- [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md)
- [Commits](https://github.com/babel/babel/commits/v7.23.3/packages/babel-core)

Updates `@babel/preset-env` from 7.17.10 to 7.23.3
- [Release notes](https://github.com/babel/babel/releases)
- [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md)
- [Commits](https://github.com/babel/babel/commits/v7.23.3/packages/babel-preset-env)

Updates `@babel/preset-react` from 7.16.7 to 7.23.3
- [Release notes](https://github.com/babel/babel/releases)
- [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md)
- [Commits](https://github.com/babel/babel/commits/v7.23.3/packages/babel-preset-react)

Updates `babel-loader` from 8.2.5 to 9.1.3
- [Release notes](https://github.com/babel/babel-loader/releases)
- [Changelog](https://github.com/babel/babel-loader/blob/main/CHANGELOG.md)
- [Commits](babel/babel-loader@v8.2.5...v9.1.3)

---
updated-dependencies:
- dependency-name: "@babel/cli"
  dependency-type: direct:development
  update-type: version-update:semver-minor
  dependency-group: babel
- dependency-name: "@babel/core"
  dependency-type: direct:development
  update-type: version-update:semver-minor
  dependency-group: babel
- dependency-name: "@babel/preset-env"
  dependency-type: direct:development
  update-type: version-update:semver-minor
  dependency-group: babel
- dependency-name: "@babel/preset-react"
  dependency-type: direct:development
  update-type: version-update:semver-minor
  dependency-group: babel
- dependency-name: babel-loader
  dependency-type: direct:development
  update-type: version-update:semver-major
  dependency-group: babel
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@afuetterer afuetterer force-pushed the update-js-dependencies branch from b28615a to 6bd56ec Compare November 29, 2023 05:52
afuetterer and others added 4 commits November 29, 2023 06:53
Dependabot couldn't find the original pull request head commit, 9d6cdbe.

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@afuetterer afuetterer force-pushed the update-js-dependencies branch from 97092cc to c5dd7e7 Compare November 30, 2023 08:54
@afuetterer
Copy link
Member Author

@MyPyDavid: Can we merge this now to take smaller steps?
This PR updates the update config and a few js libraries.

Also I added a screenshot of one of the frontend pages, like you requested.

See: https://github.com/rdmorganiser/rdmo/suites/18626099409/artifacts/1083008649

Icons are still working.

Let's continue from this save step with smaller increments and find the dependency that hides the icons. Maybe it really was react-bootstrap, which I pinned in this PR.

What do you think?

@MyPyDavid

This comment was marked as resolved.

@afuetterer afuetterer force-pushed the update-js-dependencies branch from c5dd7e7 to e1c4996 Compare November 30, 2023 15:01
@afuetterer

This comment was marked as resolved.

@afuetterer

This comment was marked as resolved.

@MyPyDavid

This comment was marked as resolved.

@afuetterer afuetterer force-pushed the update-js-dependencies branch from e1c4996 to 7d75993 Compare November 30, 2023 20:30
@afuetterer
Copy link
Member Author

@afuetterer
Copy link
Member Author

If you approve, let's merge this.

Copy link
Member

@MyPyDavid MyPyDavid left a comment

Choose a reason for hiding this comment

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

very nice, thank you!

@afuetterer
Copy link
Member Author

Thanks, took me long enough.

Let's get this into dependency-updates and then into dev.

@afuetterer afuetterer merged commit 3045403 into rdmorganiser:dependency-updates Dec 1, 2023
12 checks passed
@afuetterer afuetterer deleted the update-js-dependencies branch December 1, 2023 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file javascript Pull requests that update Javascript code type:maintenance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants