-
-
Notifications
You must be signed in to change notification settings - Fork 401
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
Simplify all extensions APIs of Context
#3456
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #3456 +/- ##
==========================================
- Coverage 44.67% 44.65% -0.02%
==========================================
Files 488 487 -1
Lines 50513 50503 -10
==========================================
- Hits 22567 22554 -13
- Misses 27946 27949 +3
☔ View full report in Codecov by Sentry. |
Test262 conformance 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.
Looks good to me :)
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.
Looks good! Just a nitpick :)
22aae73
to
3f3613d
Compare
It seems like all our extensions usages fall in the same types:
Rc
forJobQueue
andModuleLoader
.&'static
forHostHooks
.This makes our API unnecessarily complex, so we should probably just commit to those representations to simplify our API for the common use case. This also removes the lifetime parameter
Context
has, which is why there are so many changes. The important changes are: