eslint: add invenio eslint rules #551

NRodriguezcuellar · 2022-07-07T15:23:51Z

partly closes inveniosoftware/invenio-app-rdm#1789

.eslintrc.yml

package.json

ntarocco

Should be merged ASAP to avoid infinite amount of conflicts!
Ping @kpsherva @jrcastro2 @zzacharo

package.json

src/lib/DepositBootstrap.js

Now explicitly adds the fieldPath prop to field components to move away from relying on defaultProps values. related to inveniosoftware/react-invenio-deposit#551

kpsherva · 2022-07-25T09:37:18Z

src/lib/components/PublishButton/SubmitReviewModal.js

@@ -174,3 +154,7 @@ SubmitReviewModal.propTypes = {
  onSubmit: PropTypes.func.isRequired,
  initialReviewComment: PropTypes.string,
 };
+
+SubmitReviewModal.defaultProps = {
+  initialReviewComment: "",


will this result in displaying empty comments?

zzacharo · 2022-07-25T13:35:09Z

src/lib/DepositFormApp.js

@@ -112,4 +106,6 @@ DepositFormApp.defaultProps = {
  draftsService: null,
  filesService: null,
  recordSerializer: null,
+  files: undefined,


this can be described more accurately in the Proptypes e.g Proptypes.shape(...)

maybe is better to fallback to undefined always instead of null for all the missing props....Do we need to define value for optional properties actually? Can you test if removing them works? I mean the null/undefined...

Yes it is a rule that every propType that is not required should have a defaultProp, as you see i usually default to undefined so that the behaviour does not change unexpectedly.

as for setting the shape i am not sure what that could be in this case 😅

zzacharo · 2022-07-25T13:42:57Z

src/lib/components/Access/EmbargoDateField.js

+  required: false,
+  labelIcon: "calendar",
+  placeholder: i18next.t("YYYY-MM-DD"),
+  fieldPath: "access.embargo.until",


if we upper pin invenio-app-rdm and we release these changes as v0.20.x then we should remove the defaultProp fieldPath...otherwise, you need here an ignore rule no? I would also maybe add a comment saying that this will be removed in the future or similar...

Please take this into account for all the eslint ignore rules you added :)

Yes, i looked over this one, i just made sure that every fieldPath prop is set to isRequired!

maybe i should post a comment in this PR documenting why this decision (ignoring the fieldPaths defaultProps) was made and link it with every ignore?

I would start an issue to clean it out in the 0.20 maybe

issue made here #563, shall I link it in the code itself?

zzacharo · 2022-07-25T13:44:28Z

src/lib/components/AccessRightField.js

@@ -127,5 +120,8 @@ AccessRightField.propTypes = {
 };

 AccessRightField.defaultProps = {
-  fieldPath: 'access',
+  // eslint-disable-next-line react/default-props-match-prop-types
+  fieldPath: "access",


Check my comment above :)

zzacharo · 2022-07-25T13:45:22Z

src/lib/components/CommunitySelectionModal/CommunitySelectionSearch.js

@@ -129,11 +128,3 @@ export class CommunitySelectionSearch extends Component {
    );
  }
 }
-
-CommunitySelectionSearch.propTypes = {


Why is this removed?

because the components uses no props 😄

src/lib/components/DescriptionsField.js

partly closes inveniosoftware/invenio-app-rdm#1789

Now explicitly adds the fieldPath prop to field components to move away from relying on defaultProps values. related to inveniosoftware/react-invenio-deposit#551

* theme: add alias for child instance theming Signed-off-by: Karolina Przerwa <[email protected]> * release: v9.0.1 Signed-off-by: Karolina Przerwa <[email protected]> * i18n: extract messages * Logo: avoid logo image overflowing. * upgrade: add migration script from v8 to v9 * release: v9.0.2 * css: fix checkbox and avatar alignment * closes inveniosoftware/invenio-communities#690 * i18n: adds de translations * frontpage: update customize link * dashboard requests: correct searchbar behaviour now correctly holds the filter state while switching between open/closed and also when doing an empty search * members dropdown: change styling members dropdowns are now styled to resemble the RFC mockups * Invitations: fix tablet view * fix mobile view. * add status icon. * add extra space for user full name. * closes: inveniosoftware#1595 * Customize record landing page and home page. ## Record landing page. - Add record cover picture preview in side bar - Minor addition ## Integrate TU Graz recent uploads - Edit `view.py` to load and show recent uploads - Add `search.py` to query ES for recent uploads. * migrate to use black as opinionated auto formater * add .git-blame-ignore-revs * fix docs compatibilty problem with Sphinx>=5.0.0 * move check_manifest configuration to setup.cfg. concentrate the configuration of all calls in one place * increase minimal python version to 3.7 * minor: remove setuptools<=58 constraint. not necessary anymore * global: Fixes strings marked for translation. * Adds dots at the end of sentences. * Refs inveniosoftware#1707 * Note: components.js will need some more fixes later. * release: v9.0.3 Signed-off-by: Karolina Przerwa <[email protected]> * Add Keywords support in record landing-page * dashboard: add request actions * closes inveniosoftware#1215 * repair .git-blame-ignore-revs, caused by merging problems * facets: reduce negative space and align facets with results header * Load correct metadata to contributor pop-up In the deposit form, the contributor pop up dialogue was being erroneously populated with *creator* metadata. This caused the wrong roles to show where different contributor roles have been defined. * css: resize logo to fit content * closes inveniosoftware#1632 * landing page: remove excessive spacing from files table * mobile menu: add space to close button * burger menu: combine mobile and tablet visibility class * requests: don't override global request in Jinja templates * extras_require: remove elasticsearch6 * release: v9.0.4 * card: access field custom styling * closes inveniosoftware/react-invenio-deposit#519 Signed-off-by: Karolina Przerwa <[email protected]> * communities logo: set fixed height to avoid breaking the layout * landing page: prevent iframe in preview to overflow * Add remote TimeSeries resource support to LandingPage using Plotly-React. * Remove custom keywords, now use sujbects. * Configured LeafletMap and Plotly component to use record's metadata. * landing page: update funders list styling & fix tab a11y bug * site overrides: update only classes to include both block and inline-block * community header: fix alignment * searchapp: update response handler * closes inveniosoftware/invenio-communities#704 * remove hardcoded translations * global: Fixes strings marked for translation. - Introduces consistency - use plural instead of '(s)' Refs: inveniosoftware#1707 * header: add right margin to search bar * feed: fix responsive bugs for avatars * release: v9.0.5 * upload: fixed files list styling on mobile * members: reserves space for success/error icon * members: fixed alignment for icons * creatibutors: add spacing between icon and separator * A11y: Inbox icon aria label; avatar image alt tag Label for icon: "Requests" is more consistent * tables: make responsive styling apply to all stackable tables * Requests: re-design requests list view * refactor request actions. * add request actions to invenio-communites. * closes: inveniosoftware/invenio-communities#702 * Communities dashboard: re-design the view of the dashboard communities * closes: inveniosoftware/invenio-communities#701 * communities: added restricted label to community header and list. * dependencies: oauth2server, oauthclient, userprofiles, search-ui, and rdm-records * release: v9.0.6 * message overrides: update id-reference in css * eslint: add eslint rules to js files * eslint: format code * detail: add image onerror inline js to script * Landing page: Add simple IIIF image preview * Enable as default previewer for images Also adjusted IIIF_SIMPLE_PREVIEWER_SIZE to !800,800 which will prevent smaller images being stretched. * UI Filters: add has_images filter Depends on inveniosoftware#1709 * searchbar: add multiple search options * closes inveniosoftware#1701 * searchbar: add community option * define urls by flask resolver * searchbar: delete macro * global: generalize color classes and variables * header: reorganise blocks * release: v9.1.0 * config: disable pdf preview for iiif * release: v9.1.1 * landing page: fix image placeholder bug * release: v9.1.2 * bump invenio-assets * Fix deleted serialize method. * setup: upper pin werkzeug * Revert "setup: upper pin werkzeug" This reverts commit f57b98d. Upper pin was moved to invenio-base * add js/invenio_app_rdm alias * deposit form: add fieldPath prop to field components Now explicitly adds the fieldPath prop to field components to move away from relying on defaultProps values. related to inveniosoftware/react-invenio-deposit#551 * Dashboard: fix mobile pagination * Dashboard uploads: refactor uploads view * remove empty content. * create a custom mobile interface. * fix request_items naming. * closes: inveniosoftware#1771 * dashboard uploads: change buttons to dropdown on mobile Co-authored-by: Karolina Przerwa <[email protected]> Co-authored-by: Mojib Wali <[email protected]> Co-authored-by: pablo <[email protected]> Co-authored-by: Manuel Alejandro <[email protected]> Co-authored-by: Alex Ioannidis <[email protected]> Co-authored-by: jrcastro2 <[email protected]> Co-authored-by: Guillaume Viger <[email protected]> Co-authored-by: nico <[email protected]> Co-authored-by: Christoph Ladurner <[email protected]> Co-authored-by: Zeller, Christina <[email protected]> Co-authored-by: Dan Granville <[email protected]> Co-authored-by: Jenny Bonsak <[email protected]> Co-authored-by: fpistagna <[email protected]> Co-authored-by: L. Henze <[email protected]> Co-authored-by: Pablo Panero <[email protected]> Co-authored-by: Dan Granville <[email protected]> Co-authored-by: Nicola Tarocco <[email protected]> Co-authored-by: Zacharias Zacharodimos <[email protected]>

Now explicitly adds the fieldPath prop to field components to move away from relying on defaultProps values. related to inveniosoftware/react-invenio-deposit#551

NRodriguezcuellar force-pushed the add_eslint_rules branch 8 times, most recently from 6fec16d to c4ffa02 Compare July 11, 2022 15:32

kpsherva requested a review from jrcastro2 July 14, 2022 07:34

kpsherva reviewed Jul 15, 2022

View reviewed changes

.eslintrc.yml Outdated Show resolved Hide resolved

jrcastro2 reviewed Jul 15, 2022

View reviewed changes

.eslintrc.yml Outdated Show resolved Hide resolved

package.json Show resolved Hide resolved

NRodriguezcuellar self-assigned this Jul 18, 2022

NRodriguezcuellar force-pushed the add_eslint_rules branch 3 times, most recently from 88a8650 to a99281d Compare July 19, 2022 15:20

ntarocco reviewed Jul 19, 2022

View reviewed changes

package.json Show resolved Hide resolved

NRodriguezcuellar force-pushed the add_eslint_rules branch from a99281d to bfbad15 Compare July 20, 2022 08:46

NRodriguezcuellar removed their assignment Jul 20, 2022

NRodriguezcuellar commented Jul 21, 2022

View reviewed changes

src/lib/DepositBootstrap.js Show resolved Hide resolved

NRodriguezcuellar self-assigned this Jul 21, 2022

NRodriguezcuellar force-pushed the add_eslint_rules branch 2 times, most recently from 8058589 to 19173df Compare July 22, 2022 08:43

NRodriguezcuellar mentioned this pull request Jul 22, 2022

deposit form: add fieldPath prop to field components inveniosoftware/invenio-app-rdm#1811

Merged

NRodriguezcuellar force-pushed the add_eslint_rules branch from 19173df to 5aeaf20 Compare July 22, 2022 12:29

NRodriguezcuellar removed their assignment Jul 22, 2022

kpsherva requested a review from zzacharo July 25, 2022 09:05

kpsherva reviewed Jul 25, 2022

View reviewed changes

zzacharo reviewed Jul 25, 2022

View reviewed changes

NRodriguezcuellar force-pushed the add_eslint_rules branch from 5aeaf20 to 168ca9a Compare July 25, 2022 14:12

eslint: add invenio eslint rules

a0919c6

partly closes inveniosoftware/invenio-app-rdm#1789

NRodriguezcuellar force-pushed the add_eslint_rules branch from 168ca9a to a0919c6 Compare July 25, 2022 14:25

NRodriguezcuellar mentioned this pull request Jul 27, 2022

remove ignore for eslint rule when releasing 0.20 #563

Closed

kpsherva approved these changes Jul 28, 2022

View reviewed changes

kpsherva merged commit af69a46 into inveniosoftware:master Jul 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

eslint: add invenio eslint rules #551

eslint: add invenio eslint rules #551

NRodriguezcuellar commented Jul 7, 2022

ntarocco left a comment

kpsherva Jul 25, 2022

zzacharo Jul 25, 2022

zzacharo Jul 25, 2022

NRodriguezcuellar Jul 25, 2022

zzacharo Jul 25, 2022

zzacharo Jul 25, 2022

NRodriguezcuellar Jul 25, 2022

NRodriguezcuellar Jul 25, 2022

kpsherva Jul 26, 2022

NRodriguezcuellar Jul 27, 2022 •

edited

Loading

zzacharo Jul 25, 2022

zzacharo Jul 25, 2022

NRodriguezcuellar Jul 25, 2022

eslint: add invenio eslint rules #551

eslint: add invenio eslint rules #551

Conversation

NRodriguezcuellar commented Jul 7, 2022

ntarocco left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NRodriguezcuellar Jul 27, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NRodriguezcuellar Jul 27, 2022 •

edited

Loading