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

Better support for require.js #527

Merged
merged 1 commit into from
May 19, 2015
Merged

Better support for require.js #527

merged 1 commit into from
May 19, 2015

Conversation

trusktr
Copy link
Contributor

@trusktr trusktr commented May 7, 2014

I'm using Require.js, so this doesn't refer to the window object, so async didn't work until I made this change.

@trusktr
Copy link
Contributor Author

trusktr commented May 7, 2014

Just in case, this is Require.js.

@fritx
Copy link
Contributor

fritx commented May 8, 2014

failed. you should use typeof window === 'object &&' instead of window &&?

@trusktr
Copy link
Contributor Author

trusktr commented May 9, 2014

Oh thanks! Is there some way to edit the merge request? Or should I start a new one?

@fritx
Copy link
Contributor

fritx commented May 9, 2014

@trusktr you'd better start a new one ;)

@arcanis
Copy link

arcanis commented May 12, 2014

Fwiw, you just have to fix this in a new commit, then push it in the same branch. The pull request will be updated automatically.

If you really want to be clean, you can run git rebase -i before pushing, and git will ask you what to do with your commits - you will be able to merge them into one. After that, a git push -f to update the branch with the newly crafted commit, and you'll be done (the PR will be updated too).

@fritx
Copy link
Contributor

fritx commented May 12, 2014

Oh cool ! Agreed

@caolan
Copy link
Owner

caolan commented May 12, 2014

@trusktr what's happening with this?

@trusktr
Copy link
Contributor Author

trusktr commented Jan 31, 2015

@fritx @caolan Updated, finally.

@beaugunderson
Copy link
Collaborator

@trusktr Note that referencing an undeclared variable directly gives a ReferenceError, that's why the tests are failing. You'll want typeof window !== 'undefined' to check declaration without a ReferenceError. :)

@trusktr trusktr force-pushed the patch-1 branch 3 times, most recently from a4f11d5 to 786a8ca Compare February 1, 2015 01:20
@trusktr
Copy link
Contributor Author

trusktr commented Feb 1, 2015

There we go.

@trusktr
Copy link
Contributor Author

trusktr commented Feb 13, 2015

bump :)

In a RequireJS environment `this` doesn't refer to the `window` object, so async doesn't work there.
@aearly aearly changed the title Better support for browsers. Better support for require.js May 19, 2015
aearly added a commit that referenced this pull request May 19, 2015
Better support for require.js
@aearly aearly merged commit 7c7ca9b into caolan:master May 19, 2015
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.

6 participants