Skip to content

Add warning to browser recipe #1355

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

Merged
merged 1 commit into from
Apr 12, 2017
Merged

Conversation

lukechilds
Copy link
Contributor

@lukechilds lukechilds commented Apr 11, 2017

Ok so since writing browser-env I have come up with a cleaner solution, window.It's been working well for me so far and resolves some of the issues raised in #1092.

However, it requires a small code change and is only viable for smaller modules without lots of dependencies that also require a browser environment. For other modules you'll still need browser-env. For that reason I think it would be best to continue to recommend browser-env, but highlight the issues raised in #1092 and offer window as a possible alternative.

Happy to change the wording, just pretty much copied it over from what I've got in the browser-env readme.

@novemberborn
Copy link
Member

window looks neat!

I wonder if the recipe could start with that, and then lay out why (though ideal) it's not always a good fit. With the caveat that could lead up to browser-env. This way perhaps people realize window is indeed sufficient.

What do you think?

@lukechilds
Copy link
Contributor Author

Thanks :)

I considered that but decided it was best to leave as is because, while window is definitely the ideal solution, it's only viable if they aren't using 3rd party browser-only modules and if they're happy to make a slight modification to their module. I think that instantly removes a large chunk of the people that will be reading the recipe.

Also I think it could cause some confusion to less advanced users. They may try and use window not understanding why they should be using browser-env and get cryptic a is not defined errors from minified browser libs. I think browser-env should be the default because it'll always work and then the users who are able to use window can opt in.

@novemberborn novemberborn merged commit c01ac05 into avajs:master Apr 12, 2017
@novemberborn
Copy link
Member

Fair enough.

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