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

Document hooks in order, along with other relevant steps #3402

Closed
3 of 4 tasks
jakearchibald opened this issue Feb 26, 2020 · 18 comments
Closed
3 of 4 tasks

Document hooks in order, along with other relevant steps #3402

jakearchibald opened this issue Feb 26, 2020 · 18 comments

Comments

@jakearchibald
Copy link
Contributor

Documentation Is:

  • Missing
  • Needed
  • Confusing
  • Not Sure?

Please Explain in Detail...

https://rollupjs.org/guide/en/#hooks - It's currently difficult to figure out the order in which hooks are called, and how it relates to the rest of Rollup's processing.

This leads to confusion like #2739 - it isn't clear from the docs that renderChunk comes after hashing.

Your Proposal for Changes

Reorder the items in https://rollupjs.org/guide/en/#hooks so they roughly represent execution order.

If actions performed by the hook change the order, document that too.

Include internal actions, like "hashes added to file", so it's clear which things happen before/after that.

@shellscape
Copy link
Contributor

This is something that's come up before. I'm not a fan of organizing the hooks in their lifecycle order - it's confusing and I've always disliked how others do that. Alphabetic order is natural.

Instead, we should have a clear lifecycle diagram that explains how the hooks work together.

@jakearchibald
Copy link
Contributor Author

Alphabetic order is natural.

Meh, I generally find that the least useful order. Alphabetical order means you can manually binary search for something if you already know its name, which is useful for books, but computers have ctrl+f, so it's kinda redundant.

But…

we should have a clear lifecycle diagram that explains how the hooks work together

…this would work for me.

https://tabatkins.github.io/railroad-diagrams/generator.html might be useful.

@tivac
Copy link
Contributor

tivac commented Feb 26, 2020

A diagram or ordering callout is the minimal valuable change here, but I agree that I'd like to see the hooks listed in execution order instead of alphabetical.

@LarsDenBakker
Copy link
Contributor

+1 for execution order from me as well

@shellscape
Copy link
Contributor

You're all coming at this from the perspective of people who are familiar with the hooks and have been contributing to Rollup for some time. From that perspective, lifecycle order makes sense.

From the perspective a new user, a new plugin author, or someone new to Rollup attempting to debug or triage, that's a cryptic, terrible choice. Webpack's hooks docs are routinely panned for exactly this reason. Alphabetic order is far more friendly for discovery - and that needs to be the primary purpose of the documentation.

Following that, a clickable diagram at the top of the hooks section can satisfy the seasoned developer need to read those docs in that order. Hook names on that diagram can link to their place in the documentation.

@tivac
Copy link
Contributor

tivac commented Feb 26, 2020

A diagram or even a TOC-style list of links in order would be better than what we have now.

I don't agree with your assertion about discovery at all but I don't care enough to argue about it.

@jakearchibald
Copy link
Contributor Author

I don't see the benefit of alphabetical order for discovery either. If you're learning English, you don't go through a dictionary sequentially.

@shellscape
Copy link
Contributor

If you're learning English, you don't go through a dictionary sequentially.

Or Spanish or French for that matter. There are guides which teach you how to map an "other" language to English, which allows you to then reference the English word in a dictionary. And those words are easily referenced because it's in alphabetical order.

The "guide" in the example of learning a language would be the lifecycle diagram. The hooks section is an API reference, it's not a guide. Just as a list of options is a reference. We don't write documentation that lists options for a method in a "this-feels-a-certain-way" order because that'd be silly.

If you want a reference section to have a map that explains the items in the reference in a different way, that's fine. And that's essentially a diagram that explains how hooks flow and work together. A ## Hook LifeCycle section could do much the same.

@shellscape
Copy link
Contributor

This pen is exactly what I had in mind, and even fits the color scheme: https://codepen.io/philippkuehn/pen/QbrOaN

lifecycle

@jakearchibald
Copy link
Contributor Author

There are guides which teach you how to map an "other" language to English, which allows you to then reference the English word in a dictionary. And those words are easily referenced because it's in alphabetical order.

Exactly! You've demonstrated that alphabetical order is not good for discovery. The only thing it's good for is enabling the reader to perform an efficient search for an exact term they already know. Of course, that's redundant on the web, as we have ctrl+f, which performs the search much quicker than a human. The discovery part is handled by the 'guide'.

The tree diagram could work, although it might make it harder to communicate looping back to an earlier step, which happens with 'watch'.

@shellscape
Copy link
Contributor

Exactly! You've demonstrated that alphabetical order is not good for discovery.

No, I've demonstrated exactly the opposite.

Documentation should never be written from the perspective of someone who already knows its content. I could not be in a greater state of disagreement with you.

@jakearchibald
Copy link
Contributor Author

jakearchibald commented Feb 27, 2020

Documentation should never be written from the perspective of someone who already knows its content.

Right! And alphabetical order is only useful if you know the exact term you're looking for.

Let's take a more concrete example:

Using the current ordering in https://rollupjs.org/guide/en/#hooks, I know resolveId will be towards the end of the hooks section. This only works because I already know about resolveId, and haven't forgotten the letters it starts with, and I know the docs are in alphabetical order. The ordering is only useful to me because I already know the content, and the ordering is optimised for people that already know the content. Of course, I could find the content much quicker with ctrl+f.

If I was in the 'discovery' phase, and wanted to "find a way to customise how Rollup maps imports to items on disk", the alphabetical order doesn't help. Whereas, if it were in processing order, I'd figure it'd be somewhere near the top, because it'd need to happen before loading the modules.

@shellscape
Copy link
Contributor

Processing/lifecycle order only makes sense if you already know how things work. I feel like I'm taking crazy pills here - you refuse to acknowledge your bias already knowing the library, so I'm done with and unsubscribing from this conversation.

@lukastaegert ordering the hooks in lifecycle order in the docs is a massive, continent-sized thumbs down from me. I can get behind inserting a diagram at the top of the hooks section, however.

@jakearchibald
Copy link
Contributor Author

jakearchibald commented Feb 27, 2020

I feel like I'm taking crazy pills here - you refuse to acknowledge your bias already knowing the library

I also have the crazy pills feeling. You refuse to acknowledge that alphabetical order is only useful if you already know the term you're looking for (as in, you already know how things work), and either your ctrl/cmd or f key is broken.

I provided an example of discovery. It would have been nice to have seen a counter example. Oh well.

I note that the main sections of https://rollupjs.org/guide/en/ are not in alphabetical order. Good! Otherwise it would put the "Introduction" half way down the page, after "Big list of options" "Command line interface", "ES Module Syntax"… 😄

@lukastaegert
Copy link
Member

Ok please everyone, this has gotten a little out of hand. I see the one thing that everyone agrees on is that a nice lifecycle diagram with links at the top of the section will be a huge win for everyone, so that's where we should definitely go.

@jakearchibald I understand your point about alphabetical order bringing hardly any value and @shellscape I also see that you have a lot of experience with this topic from having worked on Webpack

Personal, I do not really have strong opinions either way, but I think we do not really need the ordering if we have the diagram. Maintaining a "sensible" ordering is effort and also not 100% clear for all hooks (e.g. where should renderError go, top or bottom?) which is something we have to keep reasonable both for existing and new hooks (that may be added by external contributors). Alphabetical ordering is the no-brainer ordering, at least for maintainers. That is not necessarily the "best" solution, but looking at the heated argument, maybe the one we keep for the time being as it is already in place.

But yes, we need a diagram.

@jakearchibald
Copy link
Contributor Author

Thanks for mediating @lukastaegert, and sorry @shellscape, I guess we just couldn't see each other's POV at all, so it got a little tetchy. No hard feelings, and I really appreciate all the work you do around here.

Would it be helpful to close this issue and reopen another focusing on the diagram?

Diagram UI is hard, so maybe the docs for each hook can cover what happens after that hook? As in, the docs for resolveId could end with something like:

Next lifecycle steps: This hook is called for all remaining imports in the currently processing module. Then load is called for each import in the same order.

(I don't know if the above is accurate, it's just an example)

Then, once that data is there, the community can help with creating a diagram.

@lukastaegert
Copy link
Member

Sounds like a plan. I will try to add sections like "Previous lifecycle hooks" and "Next lifecycle hooks" in the next days that link to the other hooks so that you can navigate the hooks smoothly independent of ordering. Will also try to mention conditions if there are some.

Thanks for pushing in these areas!

@jakearchibald
Copy link
Contributor Author

Closing in favour of #3409 and #3410.

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

No branches or pull requests

5 participants