nodejs / node

Node.js JavaScript runtime โœจ๐Ÿข๐Ÿš€โœจ
https://nodejs.org
Other
106.52k stars 29.03k forks source link

Re-order Node.js `src` directory ๐Ÿฅท #54363

Open Avivbens opened 1 month ago

Avivbens commented 1 month ago

What is the problem this feature will solve?

Hi Team ๐Ÿ‘‹

Lately, I've been delving into the Node.js source code quite extensively. This has been both to gather ideas for future improvements and to understand how specific features work under the hood.

One thing that came to mind is the current organization of the source files, specifically the C++ files that define the Node.js runtime functions.

Proposed Improvement

I believe it would be much better if we could organize the files instead of leaving them all in the root of the src directory. ๐Ÿ™

Think of it as the "breaking the monolith" process. Our goal is to create a well-organized codebase, complete with a clear dependencies graph, while ensuring it remains fully backward-compatible (we are not Python) ๐ŸŽ‰

I'd be happy to take on this task, if that sounds like a good idea. I understand that many people are currently working on features, so I suggest we handle this step by step.

Mitigation Strategy

We can migrate small groups of related files together, ensuring that the corresponding h and c files are kept together.

Before I proceed with showing anything, I'd appreciate knowing your thoughts on this ๐Ÿคž

Side Note

I believe the structure shouldn't be too complexโ€”ideally, no more than two levels deep. This way, we can avoid having files scattered everywhere.

What is the feature you are proposing to solve the problem?

-

What alternatives have you considered?

No response

targos commented 1 month ago

I'm not against it a priori, but it's difficult to approve before seeing at least a draft of what the new structure could be. Note that this would have to be done in small steps (not moving too many files at the same time) to avoid huge conflicts with release branches.

Avivbens commented 1 month ago

I'm not against it a priori, but it's difficult to approve before seeing at least a draft of what the new structure could be. Note that this would have to be done in small steps (not moving too many files at the same time) to avoid huge conflicts with release branches.

Absolutely, I agree. The most important aspect is to ensure that any feature development currently happening on Node.js operates smoothly and without issues.

I can work on creating a demonstration that showcases this change in a small, manageable area ๐Ÿคž

joyeecheung commented 4 weeks ago

I think the problem with a non-flat structure is that different people have different expectations about what "group" or "category" a thing should be in, and burying the files in more folders actually make them harder to discover, not easier, if you have a different mental model from who did the folder structure. I tried to do that a long time ago with some files under lib and src and TBH now I think some of that was a disservice because I myself don't even know why certain folders were named that way now (especially lib/internal/process/ and src/api/, that two folders don't make too much sense)

Avivbens commented 4 weeks ago

I think the problem with a non-flat structure is that different people have different expectations about what "group" or "category" a thing should be in, and burying the files in more folders actually make them harder to discover, not easier, if you have a different mental model from who did the folder structure. I tried to do that a long time ago with some files under lib and src and TBH now I think some of that was a disservice because I myself don't even know why certain folders were named that way now (especially lib/internal/process/ and src/api/, that two folders don't make too much sense)

I understand your perspective, but that's not quite what I had in mind.

I prefer to organize the smallest modules separately to simplify the dependencies visually. This means there will be no more than one level of directories.

I'll demonstrate this with a pull request ๐ŸŽ‰

joyeecheung commented 4 weeks ago

That was the issue that happened to src/api - there are things that one would think belong to the embedder API but they are actually universal, over time they get depended on by other files or circular dependencies start to appear and it becomes awkward unless the code gets moved around again, which disturbs git blame/cherry-pick. I think it's actually not bad to just leave the implementation where it's the most convenient and expose them to node_internals.h when another file needs them, until they get re-organized as a drive-by for a proper refactoring. Moving code for moving code's sake or splitting code for splitting code's sake can lead to a graph partitioned without enough foresight/understanding of the dependency structure.

Avivbens commented 4 weeks ago

That was the issue that happened to src/api - there are things that one would think belong to the embedder API but they are actually universal, over time they get depended on by other files or circular dependencies start to appear and it becomes awkward unless the code gets moved around again, which disturbs git blame/cherry-pick. I think it's actually not bad to just leave the implementation where it's the most convenient and expose them to node_internals.h when another file needs them, until they get re-organized as a drive-by for a proper refactoring. Moving code for moving code's sake or splitting code for splitting code's sake can lead to a graph partitioned without enough foresight/understanding of the dependency structure.

I know what you're talking about; breaking monoliths is a surgical process. But I've done that a lot ๐Ÿ˜…

I think we can show something that might fit all participants. Again, nothing fancyโ€”just a few lean, small modules to start, to avoid the current mess of 200+ files in the src root.