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.

Sign up for GitHub

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

Jump to bottom

docs: upgrade the-guild-docs version #1178

Merged
merged 10 commits into from May 6, 2022
Merged

docs: upgrade the-guild-docs version #1178

merged 10 commits into from May 6, 2022

Conversation

n1ru4l
Copy link
Collaborator

@n1ru4l n1ru4l commented May 6, 2022 •
edited

@n1ru4l
Copy link
Collaborator Author

n1ru4l commented May 6, 2022

@B2o5T Let me know if I missed something crucial

dimaMachina
dimaMachina reviewed May 6, 2022
View reviewed changes
packages/graphiql/package.json
Comment on lines +37 to +38
"react": "17.0.2",
"react-dom": "17.0.2",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Better?

Suggested change
"react": "17.0.2",
"react-dom": "17.0.2",
"react": "^17.0.2",
"react-dom": "^17.0.2",

Copy link
Collaborator

Choose a reason for hiding this comment

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

We create a static bundle before releasing this. For consistency and making sure we know the bundled version, I think it is better to go with pinned versions.

dimaMachina
dimaMachina reviewed May 6, 2022
View reviewed changes
package.json
Comment on lines +77 to +80
"@types/react": "17.0.39",
"@types/react-dom": "17.0.11",
"react": "17.0.2",
"react-dom": "17.0.2"
Copy link
Collaborator

Choose a reason for hiding this comment

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

why provide it in root package.json?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

because I don't want 900000 different versions of react installed by those packages deep in the tree that fuck up declaring loose enough dependency ranges and cause multiple instances of react runtime issues.

ardatan
ardatan reviewed May 6, 2022
View reviewed changes
Copy link
Collaborator

@ardatan ardatan left a comment

Choose a reason for hiding this comment

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

Why do we need to downgrade react versions in the examples?

@n1ru4l
Copy link
Collaborator Author

n1ru4l commented May 6, 2022 •
edited

Why do we need to downgrade react versions in the examples?

Because we don't want a lot of different react versions installed that somehow get hoisted and fuck things up.

@ardatan
Copy link
Collaborator

ardatan commented May 6, 2022

You can basically use nohoist to disable hoisting instead or resolutions to have a fixed version of react, no?

@n1ru4l n1ru4l merged commit 444f035 into master May 6, 2022
@n1ru4l n1ru4l deleted the feat-upgrade-guild-docs branch May 6, 2022 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ardatan ardatan ardatan left review comments

@dimaMachina dimaMachina dimaMachina approved these changes

@saihaj saihaj saihaj approved these changes

@charlypoly charlypoly Awaiting requested review from charlypoly

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@n1ru4l @ardatan @dimaMachina @saihaj