Skip to content
This repository has been archived by the owner. It is now read-only.

fixing #2 scope/closure problem #4

Open
wants to merge 1 commit into
base: master
from

Conversation

@ccoenen
Copy link

commented Nov 8, 2012

The wrong jsonp-function was removed (always the last one), if you did multiple calls to waveform.js in short succession. also fixes polluted window scope with jsonpN properties.

To confirm the bug:

  • make 3 or more calls to your api in very quick succession (4 calls in a for-loop is where it broke for us).
  • the last call WILL fail with a "null is not a function" or similar error.
  • you will also still have window.jsonp1 and window.jsonp2 as a reference to their functions. only window.jsonp3 (or whatever the last one is) will get nullified.

The problem is, that the variable jsonp is not a local variable but a closure. It will therefore be used as a reference for ALL jsonp-Callbacks. Thus it has the same (i.e. last) value for all callbacks. We solve this, by referencing a local variable that is individual to every jsonp-callback.

fixing #2 scope/closure problem: the wrong jsonp-function was removed…
… (always the last one), if you did multiple calls to waveform.js in short succession. also fixes polluted window scope with jsonpN properties.
@ccoenen

This comment has been minimized.

Copy link
Author

commented Nov 8, 2012

You can also see this here (original JSONP-Code)
https://github.com/IntoMethod/Lightweight-JSONP/blob/master/jsonp.js#L48
jsonp in Line 48 explicitly creates a new local variable. This was probably lost somewhere along the way to coffeescript, maybe due to the fact that jsonp is used twice (as a function name and as a local variable).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
1 participant
You can’t perform that action at this time.