Add linting and few minor cleanups

skeary commented 2 years ago

Hi,

These changes fix the peer dependencies specifiers in some of the package.json files. Previously some were (I think) incorrectly using both the >= selector as well as the ^ selector. For example:

peerDependencies": {
    "@keystone-6/core": ">=^1.0.0",
    "react": "^17.0.2"
  },

Where these occured, I changed them to remove the >=:

peerDependencies": {
    "@keystone-6/core": ^1.0.0",
    "react": "^17.0.2"
  },

I presume that if there is a major bump to keystone 6 then it's likely the contrib packages won't work - that's why I used the caret instead of the >=. I think this is the right thing to do?

I was also getting warnings in VS Code so I added linting and fixed up the issues. These were broadly:

dotenv was being used in various field packages but didn't they have a dependency on it - I've fixed this.
It looks like Keystone have changed the type BaseGeneratedListTypes to BaseListTypeInfo so I've updated the code to use the new type.
Removed the unnecessary references to the graphql package in the code so that eslint doesn't complain that it isn't a dependency.
Various minor lint errors.

I have added a lint:prettier script but I haven't included it in the lint command. It currently gives some errors and I didn't want to include prettier reformats in the changes to make them easier to review. I think it would be better if a prettier reformat was done in a separate PR but I'm happy to include them if thought worthwhile.

Could you take a look and let me know what you think of the changes?

skeary commented 2 years ago

I've noticed what I think is an error with my changes so closing for now while I resolve. Sorry for any inconvenience!

skeary commented 2 years ago

Hi, I think I've fixed the issues I mentioned. The additional changes are:

Now a consistent version of dotenv (^16.0.0) is used throughout.
Removed direct dependency on graphql-upload and made a peer dependency as that is required by Keystone core. This avoids potential issues with mixing up versions of graphql, which graphql-upload depends upon. Without doing this I was getting errors in a test project when trying to use the s3 image field reported by graphql. Given the dependency of core on graphql (via graphql-upload) I think it's safest to make graphql-upload a peer dependency?

I think this is ready to take a look at.

skeary commented 2 years ago

As requested over Slack, I've separated out the changes that fix the dependency problem in this PR into an independent PR - see https://github.com/keystonejs-contrib/k6-contrib/pull/29. The linting changes included in the PR can be done in a separate PR later if appropriate.

Closing this PR.

keystonejs-contrib / k6-contrib

Add linting and few minor cleanups #27