Skip to content

fix usage of global object in browser environment#995

Open
quanterion wants to merge 1 commit intoprotobufjs:masterfrom
quanterion:master
Open

fix usage of global object in browser environment#995
quanterion wants to merge 1 commit intoprotobufjs:masterfrom
quanterion:master

Conversation

@quanterion
Copy link
Copy Markdown

Browser code should not rely on things that are not available in browser environments. Otherwise it throws error global is undefined.

angular/angular-cli#9827 (comment)

@dcodeIO
Copy link
Copy Markdown
Member

dcodeIO commented Mar 7, 2018

The expectation here is that the sources won't be used in something else than node, while the dist files can be used in various environments. Anyway, there are multiple options to solve such issues:

  1. If a project runs in the browser, depend on the dist files instead of the sources.
  2. We could add a browser entry to package.json to automate this, but I am not sure that each and every bundler honors this.
  3. Feature-check in the node sources, possibly making bundlers with default settings pull in unnecessary / possibly large / possibly breaking shims. (That's the issue the linked comment above overlooks.)

dcodeIO added a commit that referenced this pull request Mar 7, 2018
@quanterion
Copy link
Copy Markdown
Author

It would be great to implement 2nd option. Webpack is the most popular bundler in the wild, I think it should compatible with it. I can try to make a PR if you hint me how to do that. I can't understand how to add browser entry for full and minimal libraries.

@dcodeIO
Copy link
Copy Markdown
Member

dcodeIO commented Mar 7, 2018

I can't understand how to add browser entry for full and minimal libraries.

Good point. Maybe the package should expose the dist files for everything, and just use the sources for building.

@quanterion
Copy link
Copy Markdown
Author

Yeah.. As I can see in many modern libraries like RxJS, angular, typescript etc. They all expose dist files only

dcodeIO added a commit that referenced this pull request Apr 6, 2018
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