Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

plugin not working with Rhino as is #11

Closed
jzsfkzm opened this Issue Aug 6, 2012 · 6 comments

Comments

Projects
None yet
2 participants
Contributor

jzsfkzm commented Aug 6, 2012

Hey

It's an awesome work you did with require.js and things around.
I'm playing with and SBT plugin for Jasmine running under Rhino with r.js and would like to require some files with the text plugin, but it's not loading. Thing is text plugin's createXhr method is able to return a valid object in my case and tries to load my files with an XHR call instead of the Rhino part of the long if/elseif/elseif stucture around the end of the plugin. If I replace those two parts (it looks like "if node do the node way, elseif rhino do the rhino way, elseif xhr do xhr" in that case) it works ok. Am I missing something? Should I commit it and create a pull request?

Thanks

Jozsef

Contributor

jzsfkzm commented Aug 6, 2012

One more note: I see the config option to override createXhr but r.js doesn't seem to be picking that config option up.

Owner

jrburke commented Aug 7, 2012

Yes, feel free to do a pull request that swaps the order of the checks. It is unfortunate this is needed, but seems like too many non-browser envs try to provide XHR implementations.

Owner

jrburke commented Aug 7, 2012

Closing this in favor of #12.

@jrburke jrburke closed this Aug 7, 2012

jrburke added a commit that referenced this issue Aug 8, 2012

Owner

jrburke commented Aug 11, 2012

@jzsfkzm can you give some background on the SBT plugin, maybe a link or two, and your thoughts on #13, which is caused by the change for this ticket. I'm tempted to just roll back the change for this ticket and maybe go with an explicit config you can pass to the text plugin to select an implementation override, but want to know more about SBT and how you use it, if you think that would work out. Before this change, the text plugin runs fine in plain rhino, so trying to figure out what the right set of tests are, and if there are none, then go with explicit config overrides.

Contributor

jzsfkzm commented Aug 11, 2012

The plugin I was playing with was sbt-jasmine-plugin. Feel free to revert the change, it looks like it's causing too much problems vs. fixing my problem. And sorry about that.
My original problem was Rhino is faking the XHR but then fails to load files via text.js. In theory it could be solved by creating a simple false-returning createXhr method in config but it's not working for some reason. This changeset you're about to revert - and do that! - was a workaround for that. I know we better should r.js, guess it's somewhere there broken. I will try to find the bad spot when I have the time.
Oh, and the link: https://github.com/guardian/sbt-jasmine-plugin

jrburke added a commit that referenced this issue Aug 12, 2012

Owner

jrburke commented Aug 12, 2012

Thanks for the info. I just rolled back the change for #11, you can try the latest master:

https://raw.github.com/requirejs/text/master/text.js

I also introduced a "env" config override to try to help the case for #11:

https://github.com/requirejs/text#forcing-the-environment-implemention

so hopefully that config option would help your case. If you do not think it does though, let me know and I'll remove it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment