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

Reworking the "Riverpod for Provider users" documentation section #2676

Merged
merged 40 commits into from
Jul 21, 2023
Merged

Conversation

lucavenir
Copy link
Contributor

@lucavenir lucavenir commented Jun 24, 2023

Howdies,

It's been a while since I promised "a great deal of contributions" onto the documentation... which I never did, in the end.

But not this time! I've tried my best to rework this page and address #2629.
I must say, the fact that you carefully wrote exactly what you've wanted helped a lot.

I pretty much re-structured the page as follows:

  1. Provider's limitations (an in-detail view of Provider's problems and why you won't find them in Riverpod)
  2. Towards Riverpod (how was Riverpod born, what relationship there is with Provider, what to choose)
  3. Migrating tips (as requested in the linked issue)
  4. Differences and similarities between Riverpod and Provider (which also gives migration tips under the hood)
  5. (I added a Scoping vs .family.autoDispose paragraph in this chapter)

Since I have all sort of insecurities about this, I'm expecting some sort of criticism. Let me know. Overall, I feel like this is too much content to fit into a page. I'd split this into multiple pages, one for the chapters that are now seen there

@@ -18,62 +18,179 @@ import {
This article is designed for people familiar with the [Provider] package who
wants to learn about Riverpod.

## The relationship between Riverpod and Provider
Since Provider is widely popular, why would one migrate to Riverpod?
Copy link
Owner

Choose a reason for hiding this comment

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

That probably should be a heading instead of the current "Provider's limitations"

@rrousselGit
Copy link
Owner

Thanks for this! I'll look more into this tomorrow to check everything is fine. I have a few other PRs to review first :)

@lucavenir
Copy link
Contributor Author

Perfect, take your time, I am available to implement any correction

@rrousselGit
Copy link
Owner

The page looks quite dense with those changes. There's a huge wall of text.

I wonder how it could be made more readable.

## The relationship between Riverpod and Provider
Since Provider is widely popular, why would one migrate to Riverpod?

## Provider's limitations
Copy link
Owner

Choose a reason for hiding this comment

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

This is taking too much focus on the page. Folks mainly look for migration. They probably don't care about the history behind all this. Only a minority do.

Maybe this should be moved to a devblog/article, and this section should be restricted to:

  • a simple list of the various points
  • a link to the detailed devblog

This would cleanup this page a bit, without removing information.

Copy link
Owner

Choose a reason for hiding this comment

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

Docusaurus already enables writing "articles". I've never had the need for this. But this could be a good use-case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've wrote all of these limitations in response of #2629 (see bullet points). I interpreted those bullet points as "necessary" to fix #2629. Moving "Provider's limitations" into a separate page makes sense to me

Copy link
Owner

Choose a reason for hiding this comment

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

I know. But keep in mind that #2676 is only a quick draft of what should be covered.
In the end it's quite important to make the page readable. Otherwise people will simply not read it, because they are scared of walls of texts.

I think that's generally Riverpod's issue. There's too much docs :P

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I realize this. I'm sure I can be more concise, but at the same time I wrote these subparagraphs while reminding of all the questions we're given into discord daily.

So you're telling me that on one hand folks want documentation, and on the other they don't want to read it 😆 While this is true, it all boils down to split this page as suggested below.

ALSO some examples (code) might alleviate readability. What do you think?

Copy link
Owner

Choose a reason for hiding this comment

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

The content is great, don't worry about that. I'm mainly worried about formatting, which is not something those GitHub issues care about.

Examples would for sure help. Although it could also have the adverse effect (since the page would become an even larger wall of text).

I'd combine examples with your proposal of splitting this into subpages

@lucavenir
Copy link
Contributor Author

lucavenir commented Jun 27, 2023

The page looks quite dense with those changes. There's a huge wall of text.
I wonder how it could be made more readable.

Yes, that's why I proposed to split this section into subsections.

Imo each paragraph in there deserves its own subpage. Kinda like "All Providers" has its own submenu with its subpages.

You'd obtain a main "Riverpod for Provider users" section and its subsections would be:

  • Provider's limitations
  • Towards Riverpod
  • Migrating
  • Differences and similarities

So it can be digested one bit at a time. All in all #2629 kinda requested this content, so I tried to deliver. If you approve of this @rrousselGit, I can quickly address and fix this like so.

@catalunha
Copy link

So you're telling me that on one hand folks want documentation, and on the other they don't want to read it 😆 While this is true, it all boils down to split this page as suggested below.

Congratulations to Remi and everyone from this and other PRs.
I always read and reread the Riverpod documentation. I believe that the best way to publicize/benefit is really a detailed documentation.
When learning Riverpod I made a comparative chart of the old and new syntax and when I was finishing it had already been covered in the topic
https://docs-v2.riverpod.dev/docs/about_code_generation The Syntax.
It turned out really good. Congratulations to the author of those PRs.

But there are many hidden features besides the trivial Riverpod (read/watch/notifier) that are equally important for learning and would get your readers attention.
Such as:
ref.read(xProvider.future);
ref.read(xProvider).maybeWhen();
and many others.

Now I'm studying how to exemplify the application of each one of them. But if someone already has broader knowledge it would be interesting to start any documentation about it.

In the official examples, little is said about these and other resources.

@lucavenir
Copy link
Contributor Author

lucavenir commented Jun 27, 2023

Here's my personal humble opinion. I like non-trivial, or rather "not-too-much-concise" documentation, but one must keep in mind that readability is very important. Folks expecting a documentation page can't just find an essay instead, or a didactic reading. One should find a good trade off

Imo the key is to inject more code snippets, to try to be a little more concise and to split "big pages" into sub chapters pages. I personally am very inspired by what Vue and Nuxt did in the last two years.

Let's see what Remi decides, then I'll work into it asap.

@rrousselGit
Copy link
Owner

And again, please tell me if I'm asking too much.

I don't mind continuing the work myself if you want to stop. But of course, I love the help.

@AhmedLSayed9
Copy link
Contributor

This PR is great.
It will greatly encourage and help many folks in migrating from Provider to Riverpod.

@lucavenir
Copy link
Contributor Author

And again, please tell me if I'm asking too much.

You're not. It's been a while since I promised some sort of concrete contribution, so it's time I deliver.

WIP

@lucavenir
Copy link
Contributor Author

lucavenir commented Jul 13, 2023

Ok, looks cool up until now.

Now I just need to cross reference the StateNotifier migration and we're good to go.
This means that this PR is blocked by #2630 (see my proposal there). No big deal.

Let me know if the current version looks OK for you and let me know what you want to improve 😄

@rrousselGit
Copy link
Owner

I'm working on reviewing all docs pages atm, so I have yet to look at your updates. But I just thought:

It may be worth telling users how to migrate ChangeNotifierProvider to codegen

Folks can do:

extension on Ref {
  T listenAndDisposeChangeNotifier<T extends ChangeNotifier>(T notifier) {
    notifier.addListener(notifyListeners);
    onDispose(() => notifier.removeListener(notifyListeners);
    onDispose(notifier.dispose);
    return notifier;
  }
}

@riverpod
MyNotifier example(ExampleRef ref) {
  return ref.listenAndDisposeChangeNotifier(MyNotifier());
}

I'll likely add this to the code-gen migration page too.

Back to working on docs

@lucavenir
Copy link
Contributor Author

Sure thing, let me add this in its section. I'm busy this weekend but I'll work on it asap

@rrousselGit
Copy link
Owner

No problem.

By the way since I'm trying to publish v2 docs this weekend, I may end-up editing this PR myself.

@lucavenir
Copy link
Contributor Author

lucavenir commented Jul 17, 2023

Hey there @rrousselGit,

It may be worth telling users how to migrate ChangeNotifierProvider to codegen

Done in the last commit.

I've also looked at the rework-flow branch. It looks amazing, no cap 🧢
But I am unsure how to rebase this PR with that branch (which doesn't appear in this fork).

Also, apparently I am not allowed to fork a project multiple times (which might make sense tbh).
I don't think deleting this fork is an option either, it feels way too risky 🥶

I am sorry I am not that much of a git expert, any guidance would help.

@rrousselGit rrousselGit changed the base branch from docs-v2 to rework-flow July 17, 2023 10:25
@rrousselGit
Copy link
Owner

Run:

git remote add rrousselGitRemote https://github.com/rrousselGit/riverpod
git pull rrousselGitRemote rework-flow

@lucavenir
Copy link
Contributor Author

I've just merged rework-flow into this fork. I've addressed the sidebar changes.

I've also re-arranged the final paragraphs in "motivations" to be aligned with the new flow.

Let me know what you want next from this PR. If this is okay, I'd switch to something else, such as the StateNotifier migration topic.

@rrousselGit
Copy link
Owner

Epic, thanks!

@rrousselGit rrousselGit merged commit cf666fa into rrousselGit:rework-flow Jul 21, 2023
18 of 35 checks passed
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.

6 participants