search

brianreavis / sifter.js

A library for textually searching arrays and hashes of objects by property (or multiple properties). Designed specifically for autocomplete.
1.09k stars 125 forks source link

Move microtime dependancy into devDependencies #20

Closed jkimbo closed 6 years ago

jkimbo commented 9 years ago

Can the microtime dependancy be moved into devDependencies since it's not required in the main library and it adds unnecessary node recompiles to our builds?

moonwave99 commented 9 years ago

+1

brianreavis commented 9 years ago

Not really, unless we drop the CLI utility, or at least drop the timing information from it.

jkimbo commented 9 years ago

I would be for dropping the timing information from the cli utility or even splitting out the cli into another package? I only use it as a library though so I'm biased in that regard.

alexbarrett commented 9 years ago

Just ran into compilation issues with sifter.js because of this myself. Why not just use standard JS date for timing info as you're only reporting milliseconds anyway?

gionkunz commented 9 years ago

+1 I think the problems this is causing with a forced dependency to an installed python for the Gyp bindings are outweighing the benefits of having microtime by far. optionalDependencies are the way to go here I think. I'll try to make a PR for this if that's okay.

gionkunz commented 9 years ago

This PR solves the issues https://github.com/brianreavis/sifter.js/pull/28

dfreeman commented 8 years ago

Moving microtime into the optional dependencies is somewhat helpful, but the --no-optional flag to npm install is kind of a blunt instrument when you're installing a package's dependencies from scratch and may want optional dependencies for other packages.

It's unfortunate that needing the dependency for the CLI forces users who are pulling it in as a library dependency to deal with the gyp bindings as well. It feels like pulling the CLI into its own package that has the library as a dependency would make everyone happy, though obviously that wouldn't be a small change.

ilkkao commented 8 years ago

I'd like to see this happen as well. There must be lot of people who use sifter through selectize and have to setup working C++ compiler if they want to avoid seeing npm errors in install log.

watsoncj commented 7 years ago

Since #28 was merged, I think this can be closed.

microtime is now an optional dependency, npm should continue If the dependency fails to install.

alFReD-NSH commented 7 years ago

I agree with @dfreeman, it would be better if the cli would be moved to it's own library. This will reduce the npm install time and package size for people who use this as a library as well.

brianreavis commented 6 years ago

Ended up just removing microtime altogether: https://github.com/brianreavis/sifter.js/releases/tag/v0.5.3