Migrating from StateNotifier take 2

[lucavenir]

Continues #2793, using the old rework-flow branch (I'll discard it ASAP after this PR).
Aims to fix #2630.

WIP:

• mounted is no more, and offer a migration example
• dispose vs ref.onDispose migration
• Moving from ChangeNotifier to (Async)Notifier has moved to a separate guide

[codecov]

Codecov Report

Merging #2888 (7f5f835) into docs-v2 (c9b4000) will decrease coverage by 0.01%.
Report is 208 commits behind head on docs-v2.
The diff coverage is 100.00%.

Additional details and impacted files

@@             Coverage Diff             @@
##           docs-v2    #2888      +/-   ##
===========================================
- Coverage    95.24%   95.24%   -0.01%     
===========================================
  Files           53       53              
  Lines         2252     2249       -3     
===========================================
- Hits          2145     2142       -3     
  Misses         107      107              

Files Changed	Coverage Δ
packages/riverpod/lib/src/async_notifier.dart	100.00% <ø> (ø)
packages/riverpod/lib/src/common.dart	100.00% <ø> (ø)
packages/riverpod/lib/src/notifier.dart	100.00% <ø> (ø)
.../riverpod/lib/src/async_notifier/auto_dispose.dart	100.00% <100.00%> (ø)
...od/lib/src/async_notifier/auto_dispose_family.dart	86.36% <100.00%> (ø)
packages/riverpod/lib/src/async_notifier/base.dart	100.00% <100.00%> (ø)
...ckages/riverpod/lib/src/async_notifier/family.dart	85.71% <100.00%> (ø)
...riverpod/lib/src/future_provider/auto_dispose.dart	92.30% <100.00%> (ø)
...ackages/riverpod/lib/src/future_provider/base.dart	93.75% <100.00%> (ø)
...ckages/riverpod/lib/src/notifier/auto_dispose.dart	100.00% <100.00%> (ø)
... and 12 more

[lucavenir]

There's still a few things I need feedback on before requesting for a review, and I'd love some inputs / help:

• Document how there is no-longer a "mounted" property, as a boolean wouldn't quite work (could you please write a few lines about this? I'll expand into that with some examples)
• Full-on section talking about dispose migration (same as above, some inputs please? 😟) - see this comment
• Use the <Link> component for this instead - see this comment (could you explain this to me?)
• Consistent formatting - see this comment
• Documentation about ChangeNotifier migration moved to another, separate file (WIP, but any input is appreciated)

Also I have no clue on why the pipeline failed, or rather I'm not sure how to fix coverage for a documentation-related PR

[rrousselGit]

Document how there is no-longer a "mounted" property, as a boolean wouldn't quite work (could you please write a few lines about this? I'll expand into that with some examples)

I'd suggest looking at the related issue. There's quite a lot of discussion with regards to mounted not working. Can't remember the link, but you should be able to find it

Full-on section talking about dispose migration (same as above, some inputs please? 😟) - see #2793 (comment)

I meant that overriding dispose vs ref.onDispose should be its own section. That's a separate migration step

[rrousselGit]

Use the component for this instead - see #2793 (comment) (could you explain this to me?)

Example:

riverpod/website/docs/essentials/side_effects.mdx

Line 38 in c0f4ac7

As saw previously in <Link documentID="essentials/first_request" />, we could

Consistent formatting - see #2793 (comment)

I don't use a specific tool. It's just about using the same rules. If a section is suffixed by an empty new line, all sections should :)

Documentation about ChangeNotifier migration moved to another, separate file (WIP, but any input is appreciated)

I wouldn't say it has to be a separate file (although that's fine too I guess). It's just that this is half of what people are looking after, so it's worth talking about more :)

[rrousselGit]

Also I have no clue on why the pipeline failed, or rather I'm not sure how to fix coverage for a documentation-related PR

Ignore that. The cov report tends to be flacky

[lucavenir]

Hi there again. About mounted:

I'd suggest looking at the related issue. There's quite a lot of discussion with regards to mounted not working. Can't remember the link, but you should be able to find it

I found A LOT of issues mentioning mounted, or rather the lack of it.
As I've re-read every linked issue to the mounted search keyword, here's a recap:

• mounted doesn't make sense in a (Async)Notifier; it's unreliable and it would be almost always equals to true
• The use of mounted arises because of async mutations, which may need to check if the notifier has been disposed, which is still an open problem
• It's possible to create an artificial internal _mounted property, using an Object? on startup and setting it to null on dispose
• Although possible, we can't really recommend this (because...?): we should cancel requests instead (via Completer or CancelTokens)
• This issue has been mentioned in this PR's linked issue as "related" to this mounted problem. But I can't see how
• Query mutations are often mentioned as a solution for the lack of mounted: I can't see how exactly

@rrousselGit I've applied all of the above, if you can provide feedback that'd be great.

[lucavenir]

Also, the docs are ready for a review, I'll wait for you now 😄

[lucavenir]

@rrousselGit hey, it happened again 😛 Where should this PR start from, this time?

[rrousselGit]

God dang it

From master this time. I'll review it quickly this time too, now that I merged docs-v2

[rrousselGit]

Sorry for that!

[lucavenir]

No worries (:
So... what's my best shot here? I think I might do this:

(1) Merge master into my fork (on this old branch)
(2) Create a new PR on... what branch?

@rrousselGit As soon as you tell me, I'll do it. Thank you!

[rrousselGit]

Merge master on your branch yes

The branch you use doesn't matter since you're on a fork.
But your PR should target master.

[lucavenir]

Merging master on my branch lead to 125 conflicts 😭
You should see a new PR now, but please let's close it this time, this fork is lagging behind 😛 You're sooo productive lol

EDIT. It turned out that most of those were just generated files, nvm