empiricaly / meteor-empirica-core

Core Meteor package for the experiment Empirica platform. This is where you should submit issues.
MIT License
27 stars 13 forks source link

underscore.js accidentally exposed in client-side code #240

Closed hawkrobe closed 3 years ago

hawkrobe commented 3 years ago

We're using lodash in our project and ran into a bug where we expected _. to resolve to our lodash import but it was instead resolved to the internal API's underscore import. This was difficult to track down because underscore wasn't explicitly imported anywhere in the user's project directory, only in the empirica source.

To prevent these kinds of bugs in the future, it may be better to make underscore private to the empirica class and explicitly import it in the project package.json if required in custom code? (which would, for example, allow users to switch to lodash if they wanted).

vboyce commented 3 years ago

To clarify, the error occurred before I added a lodash import line to the page (I hadn't realized it wasn't imported in that script given that it was imported and used elsewhere in the project), and led to confusion because of what type of error messages were thrown.

npaton commented 3 years ago

My apologies for the confusion. Indeed underscore is imported globally. This is a remanent of older meteor versions which made underscore globally available. Ideally we should import underscore manually everywhere it's needed, and remove the global import. I do not think this will happen in the near future as it would break too many existing experiments. We are working on a larger update that will be released later this year and will address this issue (and many others).

For the time being, I recommend you always import Lodash explicitly where the special features of Lodash are needed:

import _ from "lodash";

This should override underscore within the given file. Please let me know if this causes any issues.

You should be able to do more specific imports if you prefer, see the following guide: https://www.blazemeter.com/blog/the-correct-way-to-import-lodash-libraries-a-benchmark

hawkrobe commented 3 years ago

Perfect, thanks @npaton -- this definitely makes sense to save for a larger update with other breaking changes.