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

make search-index module browserifiable #59

Merged
merged 8 commits into from
Dec 26, 2014
Merged

make search-index module browserifiable #59

merged 8 commits into from
Dec 26, 2014

Conversation

anandthakker
Copy link
Contributor

Plan:

  • switch dependency on level to levelup and leveldown.
  • add level.js for use in browser
  • accept a db property in the SearchIndex options.
  • add search-index-browser.js that exports same API as search-index, but creating and passing in a level.js db.
  • set up the test suite to be runnable in the browser (don't know if PhantomJS has IndexedDB, or if Travis can handle other browser tests...)

Anand Thakker added 3 commits December 24, 2014 17:27
This is a first step towards making search-index fully browserifiable. See #20.

Note that currently version of levelup at the time of this writing depends on
leveldown@0.10 (not 1.0).
search-index has been changed to accept a `db` property in its options, which should
be a valid LevelUP-supported database (like level.js or memdown).

search-index-browser.js is a browserify entry point that creates a level.js db and
passes it in as an option to search-index.
The core browserify build is now working.  Try it with:

node_modules/.bin/beefy test/spec/0indexing-spec.js -- -i WNdb -i lapack

(The two -i options are to ignore unused modules
that are conditionally require()'d in NaturalNode,
but not declared as dependnecies and thus not
installed.)

Still something not quite right with
initializing the database, but getting close!
@fergiemcdowall
Copy link
Owner

This is all very sensible, and that is a good plan!

@fergiemcdowall
Copy link
Owner

There seems to be a general issue with browserifying winston (winstonjs/winston#180). I am leaning towards "fixing" this with a bunch of if (winston), but it would of course be best if there was an isomorphic logging library. Suggestions?

@anandthakker
Copy link
Contributor Author

@fergiemcdowall Oh yeah, sorry! I meant to link that winston issue in the commit log. I basically resorted to the if(winston) approach (here and here), but I set searchIndexLogger = console for the browser version so that the hacky disruption to the codebase would be localized. Happy to switch out to a different logger, but I'm not aware of one.

It occurs to me now that this could be even more localized if we moved all the searchIndexLogger setup index.js and browser.js, keeping search-index.js clean...

Anand Thakker added 5 commits December 25, 2014 17:11
Browserify (when used along with brfs transform) was failing on
these when they were in a comma-separated `var` declaration.
Just remove them since they're not being used.
Tests still failing in the browser, but they're running.
Use:

node_modules/.bin/beefy test/spec/0indexing-spec.js -- -i WNdb -i lapack

And then browse to http://127.0.0.1:9966/test/index.html
A basic example showing that indexing and searching work in the browser.  Run using beefy:

`node_modules/.bin/beefy examples/basic.js -- -i WNdb -i lapack`

and then browse to http://127.0.0.1:9966/examples/basic.html
@anandthakker anandthakker changed the title work-in-progress: make search-index module browserifiable make search-index module browserifiable Dec 26, 2014
@anandthakker
Copy link
Contributor Author

@fergiemcdowall How would you feel about merging this? I haven't gotten all the tests working, but in the last commit (ff85429), there's a simple example working in the browser. Certainly worth getting all the tests running in the browser, too, but since there's already enough here for people to start playing around, maybe that could be filed as an issue for later?

@fergiemcdowall
Copy link
Owner

Thats a really great addition. Definitely worth merging this as a starting point and then extending, hardening and documenting before the next npm release.

fergiemcdowall added a commit that referenced this pull request Dec 26, 2014
make search-index module browserifiable
@fergiemcdowall fergiemcdowall merged commit 32f9874 into fergiemcdowall:master Dec 26, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants