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

Re-order Node.js src directory 🥷 #54363

Open
Avivbens opened this issue Aug 13, 2024 · 6 comments
Open

Re-order Node.js src directory 🥷 #54363

Avivbens opened this issue Aug 13, 2024 · 6 comments
Labels
feature request Issues that request new features to be added to Node.js. lib / src Issues and PRs related to general changes in the lib or src directory.

Comments

@Avivbens
Copy link

Avivbens commented Aug 13, 2024

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

@Avivbens Avivbens added the feature request Issues that request new features to be added to Node.js. label Aug 13, 2024
@RedYetiDev RedYetiDev added the lib / src Issues and PRs related to general changes in the lib or src directory. label Aug 13, 2024
@targos
Copy link
Member

targos commented Aug 14, 2024

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
Copy link
Author

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
Copy link
Member

joyeecheung commented Aug 15, 2024

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
Copy link
Author

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
Copy link
Member

joyeecheung commented Aug 15, 2024

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
Status: Awaiting Triage
Development

No branches or pull requests

4 participants