-
Notifications
You must be signed in to change notification settings - Fork 30k
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
Update module.js #11032
Update module.js #11032
Conversation
I'm not convinced we should start removing try-catch/try-finally helper functions just yet. node v7 was the first branch to have a version of V8 that allowed for optimization of functions containing such statements and even then the functions containing these statements are currently not inlineable. |
I'm not removing try-catch/try-finally , I just adjust the sequence to delete the "threw" |
just like this return module.exports; |
You're removing the helper function ( |
In v4.x, there is no function tryModuleLoad |
The reason it's not in v4.x is because that particular change didn't get backported (due to waiting for the changes to sit for awhile in non-LTS branches). It does exist in v6.x, v7.x, and master however. |
@earforth What is the motivation for this change? We don't usually accept aesthetic-only changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'm -1 on this one. The code here is very sensitive to breakage and this does not appear to deliver enough benefit.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)