guardian / scribe

DEPRECATED: A rich text editor framework for the web platform
http://guardian.github.io/scribe/
Apache License 2.0
3.51k stars 245 forks source link

lodash requires don't match file structure of 3.5.0 #367

Closed rf- closed 9 years ago

rf- commented 9 years ago

The newest version of Scribe declares a dependency on lodash 3.5.0, but Scribe is still trying to require lodash functions using the locations they had in 2.4.1.

For example, scribe/src/element.js depends on lodash-amd/modern/collections/contains, which doesn't exist anymore. The correct path is now lodash-amd/modern/collection/contains.

rf- commented 9 years ago

The reason this doesn't break the build is that Scribe is accidentally loading a copy of lodash-amd 2.4.1 that lives in bower_components. The reason that exists is that it's still depended on by various plugins that also get installed into bower_components. There is also a copy of 3.5.0 in bower_components, but it's installed under bower_components/lodash-compat instead of bower_components/lodash-amd.

In practice, that means the built version of Scribe is still using lodash 2.4.1. However, if you try to require the Scribe NPM package using something like Webpack, Scribe breaks because it gets the copy of 3.5.0 that it actually asked for but isn't compatible with.

rf- commented 9 years ago

Updating the lodash dependency in bower.json almost fixes this, except that Bower can't install more than one version of a package at once and there are other Bower dependencies (e.g., scribe-plugin-sanitizer) that can't use 3.5.0.

rrees commented 9 years ago

Okay, I'll revert the change and start working through the dependencies, thanks for the report.

rrees commented 9 years ago

Release 1.3.4 should be back to the previous version of lodash, let me know if it's okay now.

rf- commented 9 years ago

Yep, 1.3.4 works much better, thanks! There's a commit here that updates the requires if you want to use that. It doesn't fix the build though.

rrees commented 9 years ago

Okay great, I saw the commit and I'll use it as the basis of doing the upgrade later.