search

JulianBirch / cljs-ajax

simple asynchronous Ajax client for ClojureScript and Clojure
671 stars 137 forks source link

Emitting require is problematic for the new `:bundle` target in CLJS #247

Closed bhauman closed 4 years ago

bhauman commented 4 years ago

https://github.com/JulianBirch/cljs-ajax/blob/master/src/ajax/xml_http_request.cljs#L29

Emits a require regardless of the context. So a bundler like webpack will try to resolve it, which doesn't work for a browser target.

Also, detecting nodejs via cljs.core/target is probably not the best way to do it because it is "nodejs" when using :target :bundle.

Thanks!

bhauman commented 4 years ago

OK I spent some time investigating how to solve this.

And this is what I came up with:

(cond
  (exists? goog/global.XMLHttpRequest)
  goog/global.XMLHttpRequest
  (exists? js/require)
  (let [req js/require]
    (req "xmlhttprequest")))

The main thing to to hide require so that JavaScript bundlers don't try to statically bundle xmlhttprequest in a browser environment.

The second thing is to not rely on cljs.core/*target*.

JulianBirch commented 4 years ago

Nice. Any chance of making this a PR?

On Tue, 2 Jun 2020 at 14:34, Bruce Hauman notifications@github.com wrote:

OK I spent some time investigating how to solve this.

And this is what I came up with:

(cond (exists? goog/global.XMLHttpRequest) goog/global.XMLHttpRequest (exists? js/require) (let [req js/require] (req "xmlhttprequest")))

The main thing to to hide require so that JavaScript bundlers don't try to statically bundle xmlhttprequest in a browser environment.

The second thing is to not rely on cljs.core/target.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/JulianBirch/cljs-ajax/issues/247#issuecomment-637546188, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAURE5XII2LDQC4FVYSKSTRUT5XXANCNFSM4NQCIIQQ .

bhauman commented 4 years ago

Sure, I can do that :)

On Jun 3, 2020, at 6:54 AM, Julian Birch notifications@github.com wrote:

Nice. Any chance of making this a PR?

On Tue, 2 Jun 2020 at 14:34, Bruce Hauman notifications@github.com wrote:

OK I spent some time investigating how to solve this.

And this is what I came up with:

(cond (exists? goog/global.XMLHttpRequest) goog/global.XMLHttpRequest (exists? js/require) (let [req js/require] (req "xmlhttprequest")))

The main thing to to hide require so that JavaScript bundlers don't try to statically bundle xmlhttprequest in a browser environment.

The second thing is to not rely on cljs.core/target.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/JulianBirch/cljs-ajax/issues/247#issuecomment-637546188, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAURE5XII2LDQC4FVYSKSTRUT5XXANCNFSM4NQCIIQQ .

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

call-a3 commented 4 years ago

Hi, I wanted to check in to see if there's any follow-up required. The mentioned PR https://github.com/JulianBirch/cljs-ajax/pull/248 has been made and merged, but I don't think a release has been made with this fix?

JulianBirch commented 4 years ago

Hi, please check out 0.8.1, apologies.

dfrese commented 2 years ago

Hi there, I ran into this problem with 0.8.0, but the change didn't fix it. As it seems, advanced compilation and/or in the ClojureScript version I use (0.10.733), the (let [req js/require] ...) get's optimized away. I still get a require("xmlhttprequest") in the compiled output:

ajax.xml_http_request.js
$CLJS.Sma=new $CLJS.N(null,"request-received","request-received",2110590540);$CLJS.Tma=new $CLJS.N(null,"not-initialized","not-initialized",-1937378906);$CLJS.Uma=new $CLJS.N(null,"processing-request","processing-request",-264947221);$CLJS.Vma=new $CLJS.N(null,"connection-established","connection-established",-1403749733);$CLJS.jL=new $CLJS.N(null,"response-ready","response-ready",245208276);$CLJS.kL="undefined"!==typeof $CLJS.ca&&"undefined"!==typeof $CLJS.ea&&"undefined"!==typeof $CLJS.ea.XMLHttpRequest?$CLJS.ea.XMLHttpRequest:"undefined"!==typeof require?require("xmlhttprequest").XMLHttpRequest:null;$CLJS.n=$CLJS.kL.prototype;$CLJS.n.ng=$CLJS.Qa(125);$CLJS.n.og=$CLJS.Qa(126);$CLJS.n.Ug=function(){return this.response};$CLJS.n.rg=$CLJS.Qa(122);$CLJS.n.sg=$CLJS.Qa(123);$CLJS.n.pg=$CLJS.Qa(127);$CLJS.n.qg=$CLJS.Qa(128);$CLJS.n.tg=$CLJS.Qa(124);

I tried this hack:

(let [req (atom js/require)]
      (.-XMLHttpRequest (@req "xmlhttprequest")))

and it seems to do the trick:

ajax.xml_http_request.js
$CLJS.Sma=new $CLJS.N(null,"request-received","request-received",2110590540);$CLJS.Tma=new $CLJS.N(null,"not-initialized","not-initialized",-1937378906);$CLJS.Uma=new $CLJS.N(null,"processing-request","processing-request",-264947221);$CLJS.Vma=new $CLJS.N(null,"connection-established","connection-established",-1403749733);$CLJS.jL=new $CLJS.N(null,"response-ready","response-ready",245208276);var ZG;if("undefined"!==typeof $CLJS.ca&&"undefined"!==typeof $CLJS.ea&&"undefined"!==typeof $CLJS.ea.XMLHttpRequest)ZG=$CLJS.ea.XMLHttpRequest;else{var aH;if("undefined"!==typeof require){var ona=$CLJS.Oh.a(require),bH=$CLJS.Wb(ona);aH=(bH.a?bH.a("xmlhttprequest"):bH.call(null,"xmlhttprequest")).XMLHttpRequest}else aH=null;ZG=aH}$CLJS.kL=ZG;$CLJS.n=$CLJS.kL.prototype;$CLJS.n.ng=$CLJS.Qa(125);$CLJS.n.og=$CLJS.Qa(126);$CLJS.n.Ug=function(){return this.response};$CLJS.n.rg=$CLJS.Qa(122);

Regards, David.

dfrese commented 2 years ago

Created a pull request for the "atom hack": https://github.com/JulianBirch/cljs-ajax/pull/273