Skip to content

Conversation

@ValGeorgiev
Copy link
Contributor

@ValGeorgiev ValGeorgiev commented May 5, 2021

Upgrade webpack and graphql dependencies with this PR.

I need to downgrade react-spring because of CSP issue (more details here: pmndrs/react-spring#1423)

Remove Maybe graphql type because it's internal for them now and it should not be used. (more insights here: graphql/graphql-js#2621 (comment))

There is a weird issue with our visual regression tests when I upgrade the puppeteer docker image, so I will skip that and do in another PR

Otherwise, all looks great and tested on both web and RN

@ValGeorgiev ValGeorgiev changed the title Chore/update graphql dependency chore: Upgrade webpack and graphql dependencies May 5, 2021
{
"silent": true
"silent": true,
"node": "14"
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

fs: "empty",
fallback: {
fs: false,
path: false,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Non blocking - any idea what update caused us to need to use path in the browser?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am facing this issue when I build cosmos with webpack 5
image

I will upgrade electron later and check if this is still needed

Copy link
Collaborator

Choose a reason for hiding this comment

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

Interesting - can you try building cosmos but changing this to be BUILD_ENV=extension - https://github.com/FormidableLabs/urql-devtools/blob/master/package.json#L25

I can't think of any good reason as to why we would want to use the electron version of the devtools in cosmos (other than me being a pleb and mixing up the two)

Copy link
Collaborator

@andyrichardson andyrichardson left a comment

Choose a reason for hiding this comment

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

Looks great and much needed!

If using the extension build env works and doesn't cause any issues, lets go with that - otherwise, the existing solution (adding fs/path resolutions) works fine 👍

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.

3 participants