Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace integration of Google Drive through <script> include in index.html with NPM module #109

Closed
Tracked by #249
munen opened this issue Nov 21, 2019 · 11 comments
Closed
Tracked by #249
Labels
good first issue Good for newcomers help wanted Extra attention is needed refactor Refactor existing code

Comments

@munen
Copy link
Collaborator

munen commented Nov 21, 2019

organice adds the Google Drive API SDK through an old school <script> tag in the index.html file (here).

It would be better to put it as a proper dependency through yarn add googleapis and then follow the documentation to refactor the sync backend.

Rationale:

  1. It's bad practice to add dependencies through in tags.
  2. With the current integration, it requires the browser to allow third party cookies which is proper sane to have turned off for most folks.
@munen munen added help wanted Extra attention is needed good first issue Good for newcomers refactor Refactor existing code labels Nov 21, 2019
@munen munen changed the title Replace integration of Google Drive through <script> include in index.html with NPM module Replace integration of Google Drive through <script> include in index.html with NPM module Nov 21, 2019
@munen
Copy link
Collaborator Author

munen commented May 6, 2020

Is anyone interested in taking responsiblility for the Google Drive integration? There's a number of open issues which only relate to Google Drive(#107, #109, #127).

From the current maintainers, nobody is using the Google Drive integration and nobody currently has time to spend on it. Generally speaking, the current integration works (if you're an early adopter on organice.200ok.ch or run your own instance to provide your own Google Drive API key). However, the three issues linked above prevent new users to use Google Drive easily.

It would be great if there's at least one person willing to take on proper Google Drive integration. Otherwise, I fear, that the easiest option would be to disable Google Drive until such a time comes.

If someone speaks up, I'd be absolutely willing to share information, help out, do pairing sessions (on Zoom or the like) and do code reviews.

@asymmetric
Copy link

I think considering the cost of leaving GDrive on (i.e. the necessity of allowing 3rd party scripts), it's better to remove support for it ASAP, and re-add it once someone shows up to maintain it.

@markamaze
Copy link

markamaze commented Dec 2, 2020

tried to implement googleapis npm package and ran into the same problems described in these issues:

googleapis/nodejs-dialogflow/issues/716
googleapis/nodejs-dialogflow/issues/314

@munen
Copy link
Collaborator Author

munen commented Dec 2, 2020

@markamaze Thank you for looking into this 🙏

The finding is unfortunate, though. From the first issue you're linking to, it looks like the googleapis package cannot be used from a create-react-app or next.js app at this point - maybe even not possible from an app using webpack: googleapis/nodejs-dialogflow#314 (comment)

I'm reluctant to believe that nobody using these popular frameworks is not able to use the popular Google APIs, though. Is there possibly a different package or a different integration? Tbh, I have not really looked into this a lot, but I was the person to initially post that the 'googleapis' package would be an alternative, because that's what seemed the most reasonable to me on a quick glance with regards to the problems outlined in the description of this issue.

@markamaze
Copy link

looks like gapi-script should be the right one. I'll see what I can do with it.

@munen
Copy link
Collaborator Author

munen commented Dec 3, 2020

hihi, that sounds about right - a random person bundling a package for Google.

The package doesn't have a whole lot of downloads and only 28 stars on Github. So in theory, they can MITM anything.

@markamaze
Copy link

I can see how that would be a problem.

Have been looking into what is out there and not finding much in the way of reliable packages that would simplify the problem. And looking through others' implementations of the google api in react, they're mostly in line with what is implemented in organice. I wonder if the best option is to try to figure out what google didn't like about it? There were a couple differences in the organice implementation from others I looked at, but I'm not familiar enough with it all to know if they're significant or different use cases.

I'll keep messing around with it; it's a good learning experience for me so time well spent even if I get nowhere. Hoping that I can get some feedback on where I am and what I could look into and any other suggestions/advice. Here are a few thoughts/questions I've got at the moment:

  • would it be preferable to add the gapi script to the dom from within google_drive_backend_client opposed to index.html? I did see that used elsewhere, but unless I'm misunderstanding, that still results in gapi as a global object and a dependency injected via a script?

  • I was reading about service workers, and although most of what I saw was regarding caching, the description that it operates independent of the dom, but can be setup to interact with pages within it's scope, sounded like there might be something there?

  • Am I correct that in order to try out any possible solutions, I would want to implement my own instance of the app with my own keys?

@munen
Copy link
Collaborator Author

munen commented Dec 4, 2020

And looking through others' implementations of the google api in react, they're mostly in line with what is implemented in organice.

Well, if that's the case, then let's generally continue using the script tag for the time being.

I wonder if the best option is to try to figure out what google didn't like about it?

They didn't say. Also, this is an unrelated topic to this issue. This issue is about the script tag, not about getting approval of Google for organice.200ok.ch.

would it be preferable to add the gapi script to the dom from within google_drive_backend_client opposed to index.html? I did see that used elsewhere, but unless I'm misunderstanding, that still results in gapi as a global object and a dependency injected via a script?

Since you're check concluded that using the script tag is the way to go, I think this is a great idea! I don't want to say where technically the best place is to inject the script, but it shouldn't be in index.html. This way, all users have to load the file which firstly slows down organice for all users and secondly it allows Google to track all users (maybe). It would be much better if that would only happen to users who actually want to log in with Google Drive.

I was reading about service workers, and although most of what I saw was regarding caching, the description that it operates independent of the dom, but can be setup to interact with pages within it's scope, sounded like there might be something there?

Don't know about the best practices on how to use the gapi. I'd follow the docs from them.

Am I correct that in order to try out any possible solutions, I would want to implement my own instance of the app with my own keys?

Yes. Here is documentation on how to get that running: https://organice.200ok.ch/documentation.html#org5f9253e

@markamaze
Copy link

Okay, I'll see what I can do with loading the script elsewhere.

I'll setup my own implementation and play around with it, and will be sure to bring up in the correct issue if I happen to find anything that might help with getting google to approve. I'm not holding my breath though and probably won't put too much time into it. I had hoped that implementing the googleapis npm package might help, but oh well.

@munen
Copy link
Collaborator Author

munen commented Dec 5, 2020

Sounds good!

@munen
Copy link
Collaborator Author

munen commented Jun 27, 2022

Fixed by #817

@munen munen closed this as completed Jun 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed refactor Refactor existing code
Projects
None yet
Development

No branches or pull requests

3 participants