Adjust imports in scripts and webpack directories#4739
Adjust imports in scripts and webpack directories#4739alanorth wants to merge 12 commits intoDSpace:mainfrom
Conversation
d76f3da to
0f1295b
Compare
0f1295b to
254ea5b
Compare
|
Hi @alanorth, |
|
@alanorth I'm sorry I didn't look at this sooner. It's mainly formatting and prefixes. Can you fix the conflicts and I'll review it? |
alexandrevryghem
left a comment
There was a problem hiding this comment.
Thnx @alanorth for this PR! I already took a quick look at the changed files and it looks great. I did add one small inline suggestion, but I will do a full review once those conflicts are resolved
Include the scripts and webpack directories in linting.
Automatically fix most lint errors.
254ea5b to
ff316ff
Compare
|
The scope of this PR has changed because |
Don't use require-style imports and scope some imports of path and fs so we only import methods we are using.
Disable the no-require-imports rule for copy-webpack-plugin because it does not support ES Module style imports.
Don't use a require-style import, and only import the methods we are actually using.
Don't use require-style imports, and only import the methods that we actually use instead of the whole module. Also, we need to use a different syntax to access the parsed opts after this for some reason.
cce3c97 to
452f0e2
Compare
|
I'm not sure why the lodash import rule is triggering, since we are importing individual methods: import {
isEmpty,
isEqual,
} from 'lodash';Must be something subtle about the plugin's configuration? @alexandrevryghem? |
|
@alanorth: It's part of the import isEmpty from 'lodash/isEmpty';
import isEqual from 'lodash/isEqual'; |
Don't use require-style imports, and scope the imports to only the methods we are using.
Don't use require-style imports, and only import the methods that we are actually using.
Disable eslint rule for require-style imports here since this isn't importing a Node.js module, but a command line argument.
f863722 to
4f26e4e
Compare
|
Thanks @alexandrevryghem. I've updated it. For what it's worth, I see our configuration of import isEmpty from 'lodash/isEmpty';
import isEqual from 'lodash/isEqual';While this style would be the "member" style: import {
isEmpty,
isEqual,
} from 'lodash';All of our other imports in dspace-angular use the latter style. P.S. removing the |
Related
Description
A handful of import- and console.log-related fixes were missed in the
scriptsandwebpackdirectories due to the lint configuration inangular.jsonnot considering them.Instructions for Reviewers
Please add a more detailed description of the changes made by your PR. At a minimum, providing a bulleted list of changes in your PR is helpful to reviewers.
List of changes in this PR:
angular.jsonto considerscriptsandwebpackdirectoriesnpm run lint-fixto automatically fix as many errors as possiblenode:prefix to several imports of internal Node.js APIs (fs and path) and scope to methods we useconsole.log()usage withconsole.info()Include guidance for how to test or review your PR. This may include: steps to reproduce a bug, screenshots or description of a new feature, or reasons behind specific changes.
Make sure the dev and prod builds are successful.
Make sure i18n scripts work:
Make sure all tests in CI pass.
Checklist
This checklist provides a reminder of what we are going to look for when reviewing your PR. You do not need to complete this checklist prior creating your PR (draft PRs are always welcome).
However, reviewers may request that you complete any actions in this list if you have not done so. If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!
mainbranch of code (unless it is a backport or is fixing an issue specific to an older branch).npm run lintnpm run check-circ-deps)package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.