olivernn / lunr.js

A bit like Solr, but much smaller and not as bright
http://lunrjs.com
MIT License
8.91k stars 546 forks source link

Throw proper error if non-function is added to the pipeline #308

Closed stjosh closed 6 years ago

stjosh commented 6 years ago

If someone tries to add a non-function to the pipeline (e.g., because someone typed this.pipeline.add(lunr.stopWorFilter) - not that this would ever happen to me :wink:), a rather unfriendly error is thrown: lunr.js:363 Uncaught TypeError: Cannot read property 'label' of undefined.

This commit produces a specific error and renames the warnIfFunctionNotRegistered method to checkPipelineFunction in order to reflect the slightly enhanced behaviour.

olivernn commented 6 years ago

Ah, JavaScript...

I can see what you are trying to achieve, but I'm not sold on adding runtime type checks for these kind of cases. There are many annoying things about JavaScript, this is one of them, and I don't think it is something that a library can or should solve.

To be consistent this kind of check would have to appear in every function in the API that receives parameters. Since these would happen at runtime all users would have to pay the cost of doing the runtime checks all the time, regardless of whether their code could trigger the type errors.