Skip to content

feat: transition directive wrapper for test purpose #573

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

Merged
merged 4 commits into from
Feb 4, 2025

Conversation

peterpeterparker
Copy link
Member

@peterpeterparker peterpeterparker commented Feb 4, 2025

Motivation

Svelte requires require the web animations API, and neither jsdom nor happy-dom implements it.

While in Gix-cmp or OISY we are able to apply some mocks in the configuration, this is insuffisent for the existing large test suite of NNS dapp for which many tests fail, when migrating to Svelte v5, with the error:

TypeError: Cannot set properties of undefined (setting 'onfinish')

As suggested in one of the related issues (see linked below), we can overcome the problem by disabling animations in test mode.

References

CI issues

Changes

  • Implement utils for fade, fly and scale that returns an empty transition configuration for test mode
  • Expose utils to make to re-usable by consumers as well

Tests

Errors do not happen anymore in NNS dapp - Svelte v5 wip branch running in the CI.

@peterpeterparker peterpeterparker marked this pull request as ready for review February 4, 2025 09:20
@peterpeterparker peterpeterparker requested review from a team as code owners February 4, 2025 09:20
@peterpeterparker peterpeterparker merged commit 0d2e01a into main Feb 4, 2025
9 checks passed
@peterpeterparker peterpeterparker deleted the feat/transition-wrapper branch February 4, 2025 12:02
github-merge-queue bot pushed a commit to dfinity/nns-dapp that referenced this pull request Feb 4, 2025
# Motivation

We have to disable the transition for testing purposes, as Svelte v5
introduces breaking changes in how it handles transitions. It now uses
the Web Animations API, which is not available in JSDOM or Happy-DOM.

While testing this new setup against `main`, we discovered that
disabling animations causes Svelte to use the `ResizeObserver`, which is
also an API not currently available in JSDOM or Happy-DOM (see this [CI
job](https://github.com/dfinity/nns-dapp/actions/runs/13131513035/job/36637485021)).

That is why we are mocking this API.

Worth noting, we would need to mock it anyway to integrate Svelte v5.

# References

- dfinity/gix-components#573
- dfinity/gix-components#573

# Changes

<!-- List the changes that have been developed -->

# Tests

After mocking the `ResizeObserver`, the CI passed for the branch (see PR
#6336) that was use to assert this new feature.

Signed-off-by: David Dal Busco <[email protected]>
peterpeterparker added a commit that referenced this pull request Feb 4, 2025
# Motivation

Implements the transition directives wrappers provided in #573 for the
transition that lead the
[CI](https://github.com/dfinity/nns-dapp/actions/runs/13129931873/job/36632946426)
to fail in the NNS dapp Svelte v5 branch.

# Changes

- Instead of using `svelte/transition`, use the wrappers that skips the
transitions for testing purposes

# Tests

I asserted this changes has no impact on the test suite of NNS dapp by
integrating a manual package and running the CI successfully in
dfinity/nns-dapp#6336.

We will have to mock the `ResizeObserver` though but, that's something
we need for Svelte v5 testing anyway
dfinity/nns-dapp#6337
peterpeterparker added a commit that referenced this pull request Feb 10, 2025
# Motivation

Few params types were not correctly inherited in #573.

# Changes

- Use `FlyParams` for "fly" and `ScaleParams` for "scale" instead of
`FadeParams`.
peterpeterparker added a commit that referenced this pull request Feb 10, 2025
# Motivation

Provide a wrapper for the "slide" transition, similarly as those which
were provided in #573.

Needed in OISY for Svelte v5 - i.e. looks like we will have to use those
wrappers in consuming dApps as well.

The PR also 

# Changes

- Add `testSafeSlide`.
github-merge-queue bot pushed a commit to dfinity/nns-dapp that referenced this pull request Feb 11, 2025
# Motivation

Svelte v5 requires the web animation API which is not supported in
emulated testing environment. That is why we added some "test safe
transition directive" to Gix components (see PR
dfinity/gix-components#573) and why we are using
those transitions in this PR.

# Notes

- Relates to the Svelte v5 migration PR #6020
- Requires Gix-cmp bumped in PR #6394

# Changes

- Use `testSafe...` directives instead of directly using transition of
Svelte

Signed-off-by: David Dal Busco <[email protected]>
peterpeterparker added a commit that referenced this pull request Feb 12, 2025
# Motivation

For the same reasons as #586, we can remove the transitions directives.

# Changes

- Revert #573, #579 and #580
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.

2 participants