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
WIP #38945
base: master
Are you sure you want to change the base?
WIP #38945
Conversation
…ssfull, tests require to fix before run, and fix tests too
…started to cleanup, cleanup in progress
@JustFly1984 We are reviewing this! Hope to have it merge very soon, our team is mostly in EU so it will be a few hours 🤞 |
I'm still working on it, need to clean up some stuff, will have a commit later today. |
Hi @JustFly1984, Thanks so much for all of the awesome work you've put into this Gatsby PR! We really appreciate your enthusiasm and all of improvements you've made. This PR is pretty large so it’s going to be difficult for us to review it in its current format. There are too many different features that you've worked on which is clouding our ability to understand all of the changes. We have to be mindful of regressions, additional bugs, and any security concerns so we want to ensure we can review this properly for you. To make it easier for reviewers to dive in and give you the best feedback, could we consider splitting this up into smaller pull requests? Here's what we are thinking:
For the other proposed changes:
By breaking things down a bit, we can get more focused reviews and make sure each change gets the attention it deserves. It sounds like you've been thinking a lot about how to improve Gatsby! While these changes are great, is there anything else you've been considering that you think could make Gatsby even better? We'd love to hear your ideas! Feel free to open separate GitHub issues for any additional features or improvements you have in mind. That way, we can keep track of them and discuss them with the community. We really value your contributions and insights! Thanks! P.S. Once we have these new PRs we can review and get them merged with a smaller LGTM! ;) |
…ot and packages/gatsby, also updated @react/[email protected] and @types/react-dom
I've pushed one more commit with all @ts-ignore meaningfully commented. Added missing types. |
I've updated cssnano and a lot of code in gatsby-plugin-image, pushed in last commit |
Today refactored gatsby-plugin-image gatsby-plugin-manifest and gatsby-plugin-sharp. |
@JustFly1984 can you respond to @pieh's comment? If this is to be merged we'll need you to address those issues and break these into separate PRs rather than adding more to this one |
@ascorbic @pieh I'm still working on it, typing redux. I will revert node version to 18 also. I can't really divide it into smaller PRs yet. |
The only thing I don't want to revert is pnpm. It saves so much time on package crosslinking. I've even changed docs everywhere to use pnpm instead of yarn. |
Thanks. It's great that you're doing this, but I do think it will be easier for you if you split the parts earlier, rather than after implementing everything. |
I also want to rewrite some of downstream dependencies which uses node-fetch as peerDependencies. I already got rid of node-fetch internally in gatsby repo. Today I was auditing one of my gatsby projects, and there was 5 more security issues detected. I could resolve them successfully with yarn resolutions. In general I feel it will be easier to release new major version than to divide the PR in small chunks. I've fixed a lot of stuff. |
Btw, I will bring back deprecated packages too, to reduce the change scope. |
Also I want to bring my friend to help me to rewrite xstate from 4 to 5 in 2 packages. |
We're going to need at least one PR per change, so for example one PR for the CSP, one PR for the Flow to TypeScript changes, one for Redux types etc. Do not include any of the linting or package manager changes. We can start a discussion on the change to the package manager, but that is something that cannot be done before that. I'm a big fan of pnpm, but we have a lot of tooling with yarn and lerna, so it's not something to change lightly. We're not going to change the linting or formatting rules because that means lots of changes with no real benefit. I strongly recommend you don't try to work on any other parts until we've been able to review the first PRs, or you'll need to deal with merge issues etc, as we won't be able to review each of them until the others have been merged. |
Refactoring Gatsby.
Fixing bugs:
Still require to fix:
running
pnpm outdated -r
peer dependencies issues: