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 dependencies #3082

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
Open

Conversation

david-venhoff
Copy link
Member

@david-venhoff david-venhoff commented Sep 30, 2024

Short description

This pr updates our dependencies, which includes multiple breaking changes that I attempted to fix.

Proposed changes

  • Fix typescript typechecking which for some reason did not appear to be active for preact before?
  • Replace deprecated sass functions by their non-deprecated counterparts
  • Update the sass-loader dependency to avoid another deprecation warning
  • Handle the new pylint-lint by setting the maximal amount of positional arguments to 6 (default 5) and annotating the remaining exceptions

Side effects

I also noticed 2 direct python dependencies that had a major version bump. I did some testing and don't think any functionality of the cms was broken.

Running ./tools/eslint now produces a deprecation warning. I tried updating eslint, but we depend on eslint-config-airbnb which does not support modern eslint versions. At least there is an active issue for that: airbnb/javascript#2961
I think we can accept this deprecation warning for the time being and open an issue to update eslint, but I am open to other opinions :)

Resolved issues

Fixes: /


Pull Request Review Guidelines

Breaking Changes of direct python deps:
- Cryptography:
  * Dropped support for OpenSSL < 1.1.1e
  * Dropped support for LibreSSL < 3.8
- Icalendar:
  * Use zoneinfo instead of pytz
We were using the typ `StateUpdater<_>` for setters, but the type of
`StateUpdater<T>` just resolves to `T | ((prevState: T) => T)`, which
means that it is not guaranteed to be callable.

The preact documentation is not really helpful:
https://preactjs.com/guide/v10/typescript/#typing-hooks

But in the source of `useState`, the setter is typed as
`Dispatch<StateUpdater<T>>`, so I decide to use the same type.
Due to many positives, I changed the value from 5 to 6 and
manually annotated the remaining occurrences with too many
positives, in the hopes that we will fix that at some point :)
Mypy has a better inference now, and detected a new false-positive
(I think)
deepl now returns the number of billded characters, which we
need to mimick in the fake deepl api server
Copy link

codeclimate bot commented Sep 30, 2024

Code Climate has analyzed commit f48a605 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 0.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 82.5% (0.0% change).

View more on Code Climate.

@ulliholtgrave
Copy link
Member

@david-venhoff Thank you very much! 😊 I don't think that the changes to cryptography are affecting us.
I'll do some more functional testing and probably will approve this in the next days :)

Copy link
Contributor

@charludo charludo left a comment

Choose a reason for hiding this comment

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

Wow, this looks like quite the effort, so thank you for tackling this!!
Couldn't find anything not working! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants