search

parro-it / electron-google-oauth

Google api access token in electron
MIT License
56 stars 18 forks source link

Docs example update #2

Closed Thomas101 closed 8 years ago

Thomas101 commented 8 years ago

Hey just been trying out your library and you've saved me a ton of time, thanks!

Found a bug though. If you use the example provided in the docs, the browser window params never make it down to the browser...

const googleOauth = electronGoogleOauth(browserWindowParams);

...ends up in the __unused_BrowserWindow variable field on line 71. If you pass the following arguments...

const googleOauth = electronGoogleOauth(undefined, browserWindowParams);

the right things end up in the right place. The example does have show set to false as well which means at first glance nothing seems to happen.

Thanks!

parro-it commented 8 years ago

Hi, this is intentional. When I put the BrowserWindow in first revision I was starting to work with Electron, and I thought that specifying a dependencies argument will avoid to get a dependency on Electron. But then I realize that since it can only run on Electron platform, the BrowserWindow module is available anyway.

Then, I remove the argument, and wrote the hack you mention to keep compatibility with previous release.

Do you know of other reasons to keep the BrowserWindow argument?

Thomas101 commented 8 years ago

I can't think of any. If you're only supporting electron then I don't see any reason why you can't require it directly. It might be worth adding electron to your npm peerDependencies though -- but that's only for completeness sake really.

parro-it commented 8 years ago

Ok, thank you. I leave this issue open to clarify the hack to others, just in case...

parro-it commented 8 years ago

API refactored in version 2.0.0