-
Notifications
You must be signed in to change notification settings - Fork 1
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
Initial use cases to cover #1
Comments
|
I made an example use case https://github.com/nodejs/primordials-use-cases/pull/3/files#diff-43397c849800bcb8b6ef846e1c6644ceb9b6b688c715ad0989a3d28ce9b9db09 to discuss in the meeting |
Here's how I see things: primordials are a tool that fulfils the need of "I want to access the primordial value of a V8-provided method/object/value", with "primordial" being defined as a bound value for methods, and the original value for the rest. In Node.js core, it seems to me that need comes from the following set of requirements that we set for ourselves:
I think everyone would agree that the same JS code can be written with and without primordials, the version using primordials will be less affected by userland changes (better for UX) and less readable (worse for DX) – and vice versa for the version not using primordials. Note that both UX and DX are subjective things, it's not something we can measure and each of us probably has a different opinion depending on what part of the code base we're talking about. IMO it's not "fair" to list what are the areas we want "UX over DX" (i.e. primordials), because that's the current status quo, so it applies by default everywhere. I think it would work better list the areas of the codebase where we would want "DX over UX" (e.g. Matteo was often using the example of |
@aduh95 that's why I want to use this repo to write a list of use cases for "regular people" using Node.js rather than focusng on subsystems. Those could help put what areas benefit from from the robustness than the loss of velocity. I think we can get consensus over "primordials in specific areas" (With added non-primordial related robustness like not assuming property access doesn't throw etc) like error handling and test that. In those areas we probably want to take any performance or DX losses for robustness. I think we can also get consensus over "no primordials" in certain areas that are less impacted by robustness where the usage of userland package means robustness defense against e.g. the user changing Array.prototype.push would be very minimally useful anyway. I think in order to figure out what each type of code is we need the use cases :) |
We will probably need to define what "no primordials" means, because I can forsee it being open to interpretation, but it's a discussion for another time :) Another use case: nodejs/node#32361 Maybe we could try to remove all the primordials and run a CITGM to see if any other use case arises 🤔 |
I'm not sure what the use case there is other than "I explicitly want to override Functon.prototype.bind" is though? I recall at some point a long time ago Chrome broke we override |
I think it'd be quite reasonable to decide that by opting in to a specific core module, that core module may not be robust against userland monkeypatching. However, if I have no Separately, some core modules should be robust against userland monkeypatching - Obviously we'd need to come to consensus on the specific content of those lists, but hopefully the basic concept isn't controversial? |
I think the expectation " if I have no requires or imports in my code, I would expect that nothing I do in JS can possibly break node." is reasonable but I think that includes requiring fs or child_process. Conversely |
But I think specifics would be easier to discuss once we have consensus on use cases |
Primordials should also be preferred in the prepare stack trace callback and source map handlers, for the same reason of the uncaught error handler and unhandled rejection handler. |
Some use cases from nodejs/TSC#1439 :
Inspiration for format: https://github.com/nodejs/promise-use-cases
The text was updated successfully, but these errors were encountered: