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

Non-normative layering fix for #59 #62

Merged
merged 6 commits into from
Apr 8, 2024
Merged

Conversation

guybedford
Copy link
Collaborator

@guybedford guybedford commented Apr 7, 2024

This addresses the implementation feedback in #59 that expecting an internal slot on objects that might not be defined in ECMA-262 poses implementation difficulties and that a new host hook layering would be the preferable approach to the spec boundaries.

This change satisfies the branding requirement, while resolving the implementation feedback.

The new host hook provided to implement this new check is HostGetModuleSourceName which takes a module source object and returns its string value, throwing if it is not a module source object.

The strong branding of toStringTag is then retained by calling this hook.

@guybedford guybedford changed the title Non normative change for #59 Non normative fix for #59 Apr 7, 2024
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should still present this, as even if not technically normative some delegates prefer layering changes to go throu plenary.

spec.emu Outdated Show resolved Hide resolved
spec.emu Outdated Show resolved Hide resolved
@guybedford guybedford force-pushed the non-normative-change branch 3 times, most recently from ca89a8d to 7487599 Compare April 7, 2024 23:38
@guybedford guybedford changed the title Non normative fix for #59 Non-normative layering fix for #59 Apr 8, 2024
@nicolo-ribaudo
Copy link
Member

@guybedford I was thinking about what the host implementation of this hook would look like, and it introduces much more complexity than the current spec because it forces us to answer the question of "given a user-created WebAssembly.Module not coming from the module loader, what is the corresponding module record?". While answering this question might be possible (even though we probably don't agree yet on what the answer should be, not even in the modules harmony group), it requires discussing with multiple parties.

It might be better to keep this 1:1 with the current spec and instead have a HostGetModuleSourceClassName that just returns a string. We can keep the discussion of how to build a module record from an arbitrary source in the other proposal, that will need to do it.

@guybedford
Copy link
Collaborator Author

Thanks @nicolo-ribaudo for the excellent point here. Yes let's match the existing behaviour for now and tackle the larger questions for ESM source phase layering in due course.

Copy link
Member

@lucacasonato lucacasonato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this wording is clearer.

spec.emu Outdated Show resolved Hide resolved
spec.emu Show resolved Hide resolved
spec.emu Show resolved Hide resolved
guybedford and others added 2 commits April 8, 2024 10:12
Co-authored-by: Luca Casonato <hello@lcas.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants