| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
I'm not familiar with modules code base, so maybe this suggestion doesn't apply here, but can/should we add a DCHECK before the return, so import.meta is uninitialized on Release builds, but we still check for this edge case on Debug builds? |
Sorry, something went wrong.
|
@mmarchini If we were okay with having this crash, I think the CHECK could just stay. Maybe to provide a bit of context, situations in which env == nullptr primarily target embedders (or maybe addons) that create v8::Contexts which aren’t associated with a Node.js Environment, and we don’t generally have a good way of handling that in our per-Isolate callbacks, other than maybe throwing an exception or ignoring the callback altogether. I think in this case the latter is an acceptable solution. (That doesn’t mean that we have a “good” story for the embedder + ESM situation. That would be a larger conversation, I imagine.) |
Sorry, something went wrong.
Sorry, something went wrong.
Sorry, something went wrong.
Sorry, something went wrong.
Sorry, something went wrong.
|
@addaleax said:
Is this blocking? It's not clear |
Sorry, something went wrong.
|
No, I think this is good to land. Was just trying to get a green CI first |
Sorry, something went wrong.
PR-URL: #32230 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matheus Marchini <mat@mmarchini.me>
PR-URL: #32230 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matheus Marchini <mat@mmarchini.me>
PR-URL: #32230 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matheus Marchini <mat@mmarchini.me>
PR-URL: #32230 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matheus Marchini <mat@mmarchini.me>
| Back | FazBrowse Home | New Git URL |
related commit e9fa5ae
Checklist