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

fix project references, upgrade @types/graphql #2497

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

acao
Copy link
Member

@acao acao commented Jun 9, 2022

this consolidates & simplifies the tsconfig project references tree and build steps, so that we have a proper cross-repository incremental build!

now if you run yarn tsc --watch, you'll get an incremental build of the entire repository, where only the changes you make incur a new build.

this will tide us over until we figure out how to use esbuild/vite together with references happily, or replace project references with another incremental build tool

  • consolidate project references
  • update @types/react, resolve with tsc
  • fix webpack issue with @types/react
  • add build:watch for yarn tsc --watch
  • add concurrently and yarn build:watch to start-graphiql

@changeset-bot
Copy link

changeset-bot bot commented Jun 9, 2022

⚠️ No Changeset found

Latest commit: d77fd8c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Jun 9, 2022

The latest changes of this PR are not available as canary, since there are no linked changesets for this PR.

Copy link
Collaborator

@thomasheyenbrock thomasheyenbrock left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning up some of my oversights 🙏 Do we even need to keep vite as dependency for @graphiql/react for now then? Or should we kick it out and as you said iterate over the build process for the whole monorepo later?

@acao
Copy link
Member Author

acao commented Jun 9, 2022

@thomasheyenbrock hey no worries! I wish I had more time outside of work to help review all of this! I might just apply for a graphql grant so I can keep up haha. These kinds of oversights are the kind of adaptations you need to make when the tooling isn't well documented or organized, like this repo! Also project references are a bit esoteric, especially the per-workspace references trick for determining the build order, which wasn't even documented before! Technically it's not project.json centric but this leads to some annoying boilerplate as you can see

I think we can leave it there for now, but move the vite build to build-bundles step, I just need to clean up this one webpack failure. This way, we can for now just use tsc to resolve the change for the webpack build.

There is supposed to be a way to tweak the webpack config (and likely also the vite/rollup config) where it just resolves the source files and compiles on it's own, but I think we had issues with that (led to inconsistencies between the npm package and the cdn bundle if i recall? worth a revisit!)

Also, what is the command/are the comands you're using for graphiql development?

Currently start-graphiql is supposed to be that, but I forgot that I tend to run yarn tsc --watch alongside it (all of this is half-documented :( which I should change ). Perhaps I can just add concurrently to start-graphiql?

@acao acao marked this pull request as draft June 9, 2022 14:39
@acao
Copy link
Member Author

acao commented Jun 9, 2022

Bahhh... so many errors when I try to rectify this... I apologise, this build tooling is such a house of cards.

This was referenced Aug 7, 2022
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.

2 participants