search

tfoxy / chrome-promise

Promises for chrome JavaScript APIs used in extensions and apps.
MIT License
150 stars 14 forks source link

fileSystem.retainEntry() incorrectly passed a callback? #12

Closed srsudar closed 6 years ago

srsudar commented 7 years ago

The chrome.fileSystem.retainEntry() API does not take a callback. I am seeing chromep fail when calling this method, as Chrome appears to be doing some schema checking. I can't find anything about where this happens, and I should be able to work around it, but it seems like chromep should be aware of this.

E.g. consider this code:

var id = chromep.fileSystem.retainEntry(entry);

At this point I would expect id to be a string. Instead it is a rejected Promise. From the console:

Promise {[[PromiseStatus]]: "rejected",
 [[PromiseValue]]: Error: Invocation of form fileSystem.retainEntry(object, function) doesn't match definition fileSystem.retainEntry(object entry)}

Is this expected? Is it something else about my environment that is interfering with things? Or perhaps a bug?

tfoxy commented 7 years ago

It is expected. The code of this library is very simple. It adds a callback to every function and returns a promise. It is intended to be used only with functions that have a callback.

If there was a way to detect which functions have callbacks at runtime, it should be possible to fix this. But I don't think there's a way to do it.

The only way to do this is to add the chrome api to the repo and then keep it up to date. The then-chrome library seems to have something like this, but it doesn't support the fileSystem api.

I will try to find a way to solve this. But first I need to resolve the other open issues.

srsudar commented 7 years ago

Ah great. I'm able to work around it pretty easily by overwriting the method that I need with the default method.

I think this is acceptable behavior, even if it's not ideal. If you mentioned in the the README that callers need to use the bare chrome.* APIs that don't have callbacks, I think it is perfectly fine.

I'm going to leave this open so that you can decide how you want to proceed, but as a user I'd be fine with just noting this in the documentation.

tfoxy commented 7 years ago

Yeah, I will add a warning to the readme tomorrow.

It would be great if this library can be a total replacement of chrome, so I will leave this open for now.

tfoxy commented 6 years ago

Closing this issue as there's currently no way to know if a method receives a callback or not.

srsudar commented 6 years ago

Unfortunate but seems reasonable, especially with the update to the README. Your example of using chrome/chromep is nice and clean.