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

add Bun as reserved name #6381

Merged
merged 2 commits into from
Sep 4, 2023
Merged

add Bun as reserved name #6381

merged 2 commits into from
Sep 4, 2023

Conversation

zth
Copy link
Collaborator

@zth zth commented Sep 3, 2023

https://bun.sh/ adds Bun as global object. This reserves that names to that modules cannot accidentally clash.

@zth zth requested a review from cristianoc September 3, 2023 18:58
Copy link
Collaborator

@cristianoc cristianoc left a comment

Choose a reason for hiding this comment

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

Never noticed this list before.
What's the effect in practice?

@zth
Copy link
Collaborator Author

zth commented Sep 4, 2023

That list controls if the emitted JS should prefix all interaction with a module with $$ so that referring to the module does not clash with the globally available object.

For the Bun example, if you had a module called Bun which exports only zero cost externals, things would work fine. But if you also export actual code from that module, and use that code somewhere, ReScript would import that module as Bun in the generated JS, shadowing the globally available Bun.

@zth zth merged commit d513d74 into master Sep 4, 2023
6 of 7 checks passed
@zth zth deleted the bun-keyword branch September 4, 2023 07:10
@cknitt
Copy link
Member

cknitt commented Sep 5, 2023

@zth Actually js_reserved_map.ml is a generated file, see https://github.com/rescript-lang/rescript-compiler/blob/master/CONTRIBUTING.md#update-js-reserved-keywords-map.

Could you adapt this in a way that your change is not overwritten when regenerating the file?

@zth
Copy link
Collaborator Author

zth commented Sep 5, 2023

Doh! Yeah I'll have a look at it.

@zth
Copy link
Collaborator Author

zth commented Sep 6, 2023

#6389

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