Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix audit problems by removing lodash #153

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

raphendyr
Copy link

The project doesn't really use lodash for anything beneficial. This PR removes lodash.

The some function is replaces with for and find.

The template is just removed as it's use is not documented and it didn't make sense to me. It could be potential security issue too (depending how the library user manages to config path parameter). Any case, it's not documented, so template rendering is a surprise to the user.

In addition, this is a lodash template format, i.e., /path/to/<%= ENV_VAR %>.js. At least for me, that is very strange template format for paths. I would assume shell like syntax.

The question is, does this result in a major version update or is the minor or patch enough. It's undocumented feature after all.

@@ -5,6 +5,7 @@ module.exports = {
alias: {
'my-absolute-test-lib': path.join(__dirname, 'lib/a'),
'same-folder-lib': path.resolve(__dirname, 'lib/b'),
}
},
extensions: ['', '.js']
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was my best effort to ensure the extensionsConf code path is tested. However, it's possible I broke some other relevant testing.

@@ -42,7 +42,7 @@ module.exports = (args) => {
mocks: []
}
},
exit({node}, {file}) {
exit({node}) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was left from #143

@raphendyr
Copy link
Author

@theKashey Hi, Sorry to ping you. I, and probably many others, would love if you could spare few moments in this PR. This should help to keep the project in low maintenance mode.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant