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

feat(templating): allow deactivate when activating #1729

Merged
merged 5 commits into from May 16, 2023

Conversation

jwx
Copy link
Member

@jwx jwx commented Apr 5, 2023

Pull Request

πŸ“– Description

This PR makes it possible to call deactivate on a controller while it's activating. The controller will complete the activation step it's on and then deactivate through the steps it had done so far while activating.

πŸ“‘ Test Plan

  • Make sure existing tests pass
  • Create passing tests for the issue

⏭ Next Steps

  • Remove the errors in repeaters and similar that now works

@codecov
Copy link

codecov bot commented Apr 5, 2023

Codecov Report

Merging #1729 (d1ec8ef) into master (7a2f17f) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1729      +/-   ##
==========================================
+ Coverage   87.99%   88.01%   +0.02%     
==========================================
  Files         242      242              
  Lines       22532    22555      +23     
  Branches     5156     5163       +7     
==========================================
+ Hits        19826    19852      +26     
+ Misses       2706     2703       -3     
Impacted Files Coverage Ξ”
packages/runtime-html/src/templating/controller.ts 88.19% <100.00%> (+0.57%) ⬆️

... and 2 files with indirect coverage changes

πŸ“£ We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Sayan751
Copy link
Contributor

Sayan751 commented Apr 6, 2023

For such a large change in a core component like the controller, we need some supporting tests.

@Sayan751
Copy link
Contributor

Sayan751 commented May 1, 2023

@bigopon @jwx Can you guys please hold on this PR? There might be an easier way to deal with this. I will try to submit a PR in the next few days.

@bigopon bigopon merged commit 1c9c97c into master May 16, 2023
24 checks passed
@bigopon bigopon deleted the jwx-cancel-activation branch May 16, 2023 07:17
@bigopon
Copy link
Member

bigopon commented May 16, 2023

Thanks @jwx @Sayan751 nice work!

AureliaEffect pushed a commit that referenced this pull request May 21, 2023
**BREAKING CHANGE:**

* **compiler:** avoid using au class to find targets (#1768) ([0d30998](0d30998)).
* ***:** rename resolveAll -> onResolveAll (#1764) ([fdf0747](fdf0747))

**Features:**

* **di:** ability to use newInstance()/forScope() with interface (#1767) ([a0d39e9](a0d39e9))
* **all:** allow injection of implementation (#1766) ([a60db13](a60db13))
* **templating:** allow deactivate when activating (#1729) ([1c9c97c](1c9c97c))
* **bindable:** support getter/setter (#1753) ([4279851](4279851))
* **ui-virtualization:** enhance implementation & publish package (#1759) ([7a2f17f](7a2f17f)). Thanks [@Lakerfield](https://github.com/Lakerfield)

**Refactorings:**

* ***:** cleanup up unused code & decouple interface from default impl (#1761) ([7a71d43](7a71d43))
* **router:** add warning for unsupported behavior (#1757) ([ce87339](ce87339)). The router used to allow container traversal to
find resources when a string is used as a route to find the corresponding component.
Going forward this will be invalid and resources must be registered either locally
or globally to be routable. A warning is added first so app that accidentally used
this behavior can detect the invalid usages and fix accordingly first. This behavior will be removed in a near future release.
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.

None yet

3 participants