Replace keypather with lodash.get

[lfdebrux]

keypather is a bit unloved and Dependabot complains about it. We can replace it with the _.get function from lodash [1]. This also saves a little bit of disk space, as we have to install lodash loads anyway.

This PR checks that everything is a-okay by adding some small tests for checked, which uses keypather, and then doing the switch.

[domoscargin]

This looks great and seems hunky dory on my machine.

Do we think this could break some existing prototypes where users have made use of keypather elsewhere?

[lfdebrux]

Do we think this could break some existing prototypes where users have made use of keypather elsewhere?

Hmm, possibly... it would be nice if we could find if there were any such prototypes on GitHub 🤔

[domoscargin]

Do we think this could break some existing prototypes where users have made use of keypather elsewhere?

Hmm, possibly... it would be nice if we could find if there were any such prototypes on GitHub 🤔

Only 1500ish results to check... https://github.com/search?l=JavaScript&q=%22keypather%22&type=Code

[joelanman]

we've never documented keypather or told people to use it/aware of people using it, in which case we don't normally consider it part of our 'api' to be included in 'breaking changes'

[domoscargin]

Given @joelanman's comment that we don't consider this a breaking change, I think this is ready to go with a fix entry in the CHANGELOG. dependabot is going to be so pleased.

[lfdebrux]

I've update the CHANGELOG👍 When do we want to merge this?

[domoscargin]

There shouldn't be any harm in merging straight away?

We could potentially do a 11.0.1 release with this so anybody who's upgrading now doesn't get the Big Scary Security Warning?

[joelanman]

@domoscargin sounds good to me on both counts

[fofr]

As a prototype developer that's re-used keypather in the kit, I'd like to know a bit more about this change.

I like that this fixes the security issue, and lodash looks like a good replacement - is the API similar, and is it pretty much a straight swap?

[domoscargin]

Hi @fofr - we haven't gone into much detail on this in the release notes, since we only used keypather to help out in a utility function and don't promote it for users:

govuk-prototype-kit/lib/utils.js

Line 5 in 266226c

const { get: getKeypath } = require('lodash')

We were specifically interested in get for our case, and we were able to simply swap out keypather for lodash.get with no code changes, but with two bonuses: we got rid of the security vulnerability, and we were able to add lodash to our package.json - it was already a sub-dependency multiple times, and it's a useful library that some of our users make use of.

If you're making use of other keypather functions, or using it in a more advanced way, it'd definitely be worth checking.

At a glance:

lodash doesn't provide the options argument, but otherwise get, set, unset (it's version of del) are largely compatible.

For immutable actions, looks like the lodash/fp module might help.

has and hasIn would be the lodash analogues for keypather's has and in.

For flatten and expand, looks like a bit more work is involved.