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

Define browser main for esbuild-wasm #250

Merged
merged 1 commit into from
Jul 12, 2020
Merged

Conversation

guybedford
Copy link
Contributor

This ensures bundlers and tools will properly support a browser build.

This can be done with "exports" too, but probably makes sense to use main / browser for a few months yet at least until Node.js 12 support can be assumed.

@evanw
Copy link
Owner

evanw commented Jul 11, 2020

Thanks for taking the time to send a PR for this. However, I deliberately removed this recently. The esbuild APIs for node and the browser are different, and overlaying them like this is confusing in my opinion. It also prevents type checking from working properly because TypeScript currently doesn't have a way of selecting another main field (see microsoft/TypeScript#21423). I value having type checking for the API because I believe that IDE autocomplete is a good form of documentation. I really wish there was another way to make this work because I agree that having to import esbuild-wasm/lib/browser is aesthetically less than ideal, but type checking and API consistency seemed more important. What do you think?

@guybedford
Copy link
Contributor Author

guybedford commented Jul 12, 2020 via email

@evanw
Copy link
Owner

evanw commented Jul 12, 2020

There may be better ways to deal with the typing split though. Are the APIs not similar? Could the types ensure they provide a universal subset that works for both?

The APIs are similar but different. A common subset would eliminate all of the function declarations, making the type definitions useless (there's no API call that's the same in both APIs).

I suppose I could give up on accurate type information. I could turn both the node and browser APIs into a single API that is the union of both, and then "disable" certain API calls at run time by throwing exceptions when you make an API call from the other platform.

@evanw evanw merged commit 92cfbd1 into evanw:master Jul 12, 2020
@guybedford guybedford deleted the patch-1 branch July 12, 2020 11:38
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.

2 participants