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

chore: Upgrading dependencies #21

Merged
merged 15 commits into from
Sep 11, 2023
Merged

chore: Upgrading dependencies #21

merged 15 commits into from
Sep 11, 2023

Conversation

Tohaker
Copy link
Collaborator

@Tohaker Tohaker commented Sep 8, 2023

Notable changes;

  1. @tsconfig/node18 upgraded from v1.0.1 to v18.2.0 to match their new versioning convention. This meant I had to change out some module compiler options as commonjs isn't needed anymore. Tested the UI and connections still work.
  2. The new tsconfig meant that the Pothos types threw some errors, but the fix for this was to explicitly type Objects in the schema builder, nothing too untidy.

Other various upgrades include the build tools, type definitions, and some UI plugins. I ran through all the tests locally, and did a cursory manual check to make sure the UI still built and displayed as before.

Copy link
Member

@danstarns danstarns left a comment

Choose a reason for hiding this comment

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

This works well for me and in the pipelines - some big upgrades there 👌

Looking to adopt something in the pipelines for this so we can always keep up to date:

#23

"eslint-plugin-n": "^16.0.0",
"eslint-plugin-prettier": "^5.0.0",
"eslint-plugin-promise": "^6.1.0",
"eslint-plugin-react-hooks": "^4.6.0",
Copy link
Member

Choose a reason for hiding this comment

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

I am curious to hear your thoughts @Tohaker on pinning vs. nonpinning dependencies in this project.

Should we pin everything? Should we pin some and not others?

Copy link
Member

Choose a reason for hiding this comment

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

With the addition of #23 could we pin everything and then rely on that to apply patches etc.?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I pin dependencies and leave devDependencies unpinned. That way we control what's released to users but allow developers to have the patched versions of what they need to work with.

However, once we have an automated process bumping dependency versions whenever they're available, I'm comfortable to have everything pinned.

Copy link
Member

Choose a reason for hiding this comment

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

I like the idea of pinning the main ones and keeping the dev ones floating.

Also, something to note here is that we have one expectation in the mono repo and that

peerDependencies
    "graphql": "^16.0.0 || ^17.0.0"

In the utils package.

Keen to hear thoughts on if this is the best approach to incude GraphQL in your project without clashing with other versions:

Cannot use GraphQLSchema "[object Object]" from another module or realm.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think that should work in theory, although when I tried adding this one locally to my own existing (albeit far out of date) project it threw the error above. Might need to triage whether that's a setup issue we have or if it was just my crummy code 😆

e2e/package.json Show resolved Hide resolved
@Tohaker Tohaker merged commit adaabbe into main Sep 11, 2023
2 checks passed
@Tohaker Tohaker deleted the chore/upgrading-dependencies branch September 11, 2023 20:54
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.

None yet

2 participants