Clarify Enhance Function
Closed this issue · 5 comments
The enhance function example on the README returns item, which is irrelevant as the return value is never referenced (see snippet below). Instead, one needs to mutate item directly.
Can you please clarify what the intended functionality is here? I was attempting to modify item with a return value, but it was having no affect.
i.e. the following would not change the path on item:
new DirectoryTreePlugin({
dir: './src/content',
path: './src/_content.json',
extensions: /\.md/,
enhance: (item, options) => {
return {
...item,
path: item.path.replace(options.dir, '')
}
}
})In order for it affect the outcome, you'd need to capture the return value. Something like:
- if ( enhance ) enhance(item, allOptions)
+ if ( enhance ) item = enhance(item, allOptions)Good catch, the documentation is incorrect. Feel free to submit a PR to correct it, which could go one of two ways...
- Update the docs to remove the
returnbit and clarify that users ofenhanceshould modifyitemin place. - Make the modification you suggest in that final diff above to
src/index.js.
Note that option 2 would be a breaking change so we'd have to cut the next release as a new major. Personally I'm not really using this package anymore so I don't have a much of a preference one way or another. That said, I'm happy to review any pull requests and cut new releases as needed.
Personally I'm not really using this package anymore so I don't have a much of a preference one way or another.
Are you still actively maintaining this repo with dependency/security updates? This package was the least stale of the options I could find, and saw that the webpack/webpack.js.org repo was a dependant of this package, so thought it'd be a safe bet.
Regardless, I'll submit a PR, but I'm going to determine if I need option 2 first. If not, updating the README is less work :)
Are you still actively maintaining this repo with dependency/security updates?
Yes, I merge anything dependabot submits and I'm happy to cut new versions whenever prompted. Looks like there's a couple security warnings it didn't fix automatically though now that you bring it up so I'd be happy to accept a PR for those otherwise I can try to address them this weekend.
This package was the least stale of the options I could find, and saw that the webpack/webpack.js.org repo was a dependant of this package, so thought it'd be a safe bet.
Yeah, so I'd put it like this... I wrote this when I was leading the development on webpack.js.org and given that it's still being used there that's a decent indication of its ability to solve the problem. That said, I think it can be improved and gatsby-source-filesystem may be a good source of inspiration (though I think it should just expose the JSON structure but not force the use of GraphQL).
More specifically, I'm not sure the plugin should be generating a file that's explicitly used in the dependency tree (something about this has just always felt funny to me but I can't put my finger on what). There may be other options like injecting a global variable. Hope this context helps!
Just addressed the security warnings. Lmk what you decide and we'll figure out when to get the next version shipped.