change to curl() API: `curl(string)` should be sync #144

Open
unscriptable opened this Issue Oct 23, 2012 · 8 comments

Comments

Projects
None yet
4 participants
@unscriptable
Member

unscriptable commented Oct 23, 2012

I'm proposing that we change the curl() API behavior a bit. When debugging, it's nice to be able to check the value of a module (or plugin-base resource) without having to create a callback function. There's no way to do this today; you always need a callback function.

At the moment, the following two function signatures evoke the same behavior. curl() detects a string in the first param and converts it to an array.

curl('my/module', function (mod) { console.log(module); });
curl(['my/module'], function (mod) { console.log(module); });

As long as the dev doesn't provide a callback function, we can assume that the dev meant to use curl() as an r-value (sync).

In code, this means the following will work:

var mod = curl('my/mod');

This will also work in the console:

curl('my/mod'); // shows the module in the console

When using this signature, if the module isn't already loaded, curl() will throw.

Hey @asilvas, I know you've shown some code snippets showing a single string param that assumed async behavior. How do you feel about changing this? -- John

@ghost ghost assigned unscriptable Oct 23, 2012

@asilvas

This comment has been minimized.

Show comment
Hide comment
@asilvas

asilvas Oct 24, 2012

Contributor

I really like the idea of returning the single module -- I seem to remember requesting something similar a ways back. However, I would avoid throwing an exception, as that'd be a breaking change. In (rare) cases, modules are loaded without concern of when they're available (examples: preloading modules, styles, or bundles) -- in your example, this would now throw an error, which is bad. I'd recommend simply returning undefined.

You could even take it a step further, and account for arrays as well.

var mod = curl('my/module');
var mods = curl(['loadedModule', 'notYetLoadedModule']);
mods == [ module, undefined ]
mod = curl(['singleModuleInArray']); // single item array is not returned in array
Contributor

asilvas commented Oct 24, 2012

I really like the idea of returning the single module -- I seem to remember requesting something similar a ways back. However, I would avoid throwing an exception, as that'd be a breaking change. In (rare) cases, modules are loaded without concern of when they're available (examples: preloading modules, styles, or bundles) -- in your example, this would now throw an error, which is bad. I'd recommend simply returning undefined.

You could even take it a step further, and account for arrays as well.

var mod = curl('my/module');
var mods = curl(['loadedModule', 'notYetLoadedModule']);
mods == [ module, undefined ]
mod = curl(['singleModuleInArray']); // single item array is not returned in array
@unscriptable

This comment has been minimized.

Show comment
Hide comment
@unscriptable

unscriptable Oct 31, 2012

Member

Are you currently using curl('my/module') instead of curl(['my/module']) in your project(s)?

Member

unscriptable commented Oct 31, 2012

Are you currently using curl('my/module') instead of curl(['my/module']) in your project(s)?

@dizzib

This comment has been minimized.

Show comment
Hide comment
@dizzib

dizzib Nov 8, 2012

This is a very good idea. Currently my unit and integration test code has unnecessary async complexity :(

dizzib commented Nov 8, 2012

This is a very good idea. Currently my unit and integration test code has unnecessary async complexity :(

@briancavalier

This comment has been minimized.

Show comment
Hide comment
@briancavalier

briancavalier Nov 8, 2012

Member

Seems useful to me to return the module, or array of modules. I'm not sure about returning undefined vs. throwing, tho. Returning undefined would mean that it's harder to distinguish between a module that couldn't be loaded, and a module that was loaded, but returned undefined (either intentionally, which is probably rare, or unintentionally, meaning probably a bug). That may not be a problem in practice tho.

I do think tho that if an array is passed to curl(), it should always return an array, even in the case where the array contains a single item. It seems like this consistency is intuitive: if you pass in an array, you always receive an array, if you pass in a string, you always receive a single module.

Member

briancavalier commented Nov 8, 2012

Seems useful to me to return the module, or array of modules. I'm not sure about returning undefined vs. throwing, tho. Returning undefined would mean that it's harder to distinguish between a module that couldn't be loaded, and a module that was loaded, but returned undefined (either intentionally, which is probably rare, or unintentionally, meaning probably a bug). That may not be a problem in practice tho.

I do think tho that if an array is passed to curl(), it should always return an array, even in the case where the array contains a single item. It seems like this consistency is intuitive: if you pass in an array, you always receive an array, if you pass in a string, you always receive a single module.

@asilvas

This comment has been minimized.

Show comment
Hide comment
@asilvas

asilvas Nov 8, 2012

Contributor

@unscriptable Missed your last question. We use both string for single module and array of string's where applicable. I'd say single string is typical in over 50% of our use cases.

Contributor

asilvas commented Nov 8, 2012

@unscriptable Missed your last question. We use both string for single module and array of string's where applicable. I'd say single string is typical in over 50% of our use cases.

@asilvas

This comment has been minimized.

Show comment
Hide comment
@asilvas

asilvas Nov 8, 2012

Contributor

Being able to do things like this would be a handy side-effect:

 if (curl("module")) { // our resource is ready
 }

Another use-case, which currently would not work using the above calling convention, would be to detect for the existence of the object without triggering it's loading. Such as:

if (curl("exists!module")) { // available, but will not auto-load if not available
}

I don't have any specific need for this feature, just thought I'd mention it as it'd be a nice addition.

Contributor

asilvas commented Nov 8, 2012

Being able to do things like this would be a handy side-effect:

 if (curl("module")) { // our resource is ready
 }

Another use-case, which currently would not work using the above calling convention, would be to detect for the existence of the object without triggering it's loading. Such as:

if (curl("exists!module")) { // available, but will not auto-load if not available
}

I don't have any specific need for this feature, just thought I'd mention it as it'd be a nice addition.

@unscriptable

This comment has been minimized.

Show comment
Hide comment
@unscriptable

unscriptable Nov 8, 2012

Member

curl("exists!module") would be very easy to create, actually.

Member

unscriptable commented Nov 8, 2012

curl("exists!module") would be very easy to create, actually.

@asilvas

This comment has been minimized.

Show comment
Hide comment
@asilvas

asilvas Nov 8, 2012

Contributor

Could you get it to work with other plugins as well?

curl("exists!js!myscript.js")

or would it be?

curl("exists!myscript.js")

Contributor

asilvas commented Nov 8, 2012

Could you get it to work with other plugins as well?

curl("exists!js!myscript.js")

or would it be?

curl("exists!myscript.js")

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