search

algolia / github-awesome-autocomplete

:octocat: Add instant search capabilities to GitHub's search bar
https://github.algolia.com
Other
1.07k stars 93 forks source link

Browserify dependencies #25

Closed lautis closed 8 years ago

lautis commented 9 years ago

This is pretty much yak shaving: I wanted to port this plugin to Safari, but Hogan templates can't be compiled live. Safari applies GitHub CSP is also to extensions (at least for inline-eval, but algolia search requests seem to be allowed). Additionally, Safari appears to load extension JS asynchronously, which means that content.js might be running before other scripts are fully executed.

To solve these issues without breaking existing plugins, I migrated the templates and dependencies to be included via Browserify. The compiled content.js seems to run in Safari as well as other browsers.

I also updated the dependencies as older versions of libraries weren't compatible with Browserify. This could've broken something. I wasn't able to test the extension as signed in user as private repository crawling seems to take quite a while for my account.

redox commented 9 years ago

Wow, that's great! I started to look at the Safari port a few months ago but never took the time to nail it.

Using browserify everywhere is probably the right way to go. I'll check how it looks like in Chrome & Firefox later this week. I could also try to cleanup our code, kind of messy right now.

My only concern is maybe with the Firefox's moderation, they don't like minified/hidden/concatenated dependencies so I hope using browserify is OK for them.

lautis commented 9 years ago

Browserify might not be needed on Firefox as it seems to support npm modules since 38.0 with jpm. Not sure if it actually would work, though.

On the other hand, Firefox AMO review policies state that

Add-ons may contain binary, obfuscated and minified source code, but Mozilla must be allowed to review a copy of the human-readable source code of each version of an add-on submitted for review.

I'd interpret this as having a repo that can be used to produce the exact copy of the addon bundle would be enough. Shrink-wrapping dependencies could help in that, and perhaps it wouldn't be bad to produce more reliable builds in general.

redox commented 9 years ago

Oh, I didn't know about jpm, that's good to know.

I'd interpret this as having a repo that can be used to produce the exact copy of the addon bundle would be enough.

That's also what I initially thought, but I ended up using non-minified+included dependencies to convince them :)

lautis commented 9 years ago

By the way, I've pushed the Safari branch to https://github.com/lautis/github-awesome-autocomplete/tree/safari. It works at least without OAuth, but I seem to get a timeout when accessing https://github.algolia.com/signin.

redox commented 8 years ago

I'm gonna close this PR but I've taken some of your ideas in https://github.com/algolia/github-awesome-autocomplete/commit/0a717b05fe2b6be3ad9d418910ced32a60e65939. We definitely need to upgrade the underlying algoliasearch deps, but it's actually a patched version (that removes the JSONP capabilities for security reason). I'll also replace typeahead by autocomplete.js anytime soon.