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

[Blocks Fixture] fix broken fixture ServerComponent app #22151

Closed

Conversation

ryota-murakami
Copy link
Contributor

@ryota-murakami ryota-murakami commented Aug 21, 2021

Summary

Since removed 'react/unstable-cache' entry point by above PR, Blocks not working with error on the next line.

import {createCache, CacheProvider} from 'react/unstable-cache';

Test Plan

I've planning adding test by Cypress.

Screen Capture

Screen.Recording.2021-08-22.at.4.17.49.mov

@ryota-murakami
Copy link
Contributor Author

@gaearon I think you are best reviewer as a original author.
Please feel free to feedback when if you have a plenty time! 😄

@bvaughn
Copy link
Contributor

bvaughn commented Aug 22, 2021

Not sure why the replace-fork CI fails for non-reconciler stuff like this. Looks like our script is broken.

@ryota-murakami
Copy link
Contributor Author

@bvaughn Hello Brian👋 This is Sunday so could you have plenty rest time? 😀

I don't know this CI background though, looks like there is diff on git between original and fork.
git diff --quiet || (echo "Reconciler forks are not the same! Run yarn replace-fork. Or, if this was intentional, disable this CI check." && false)
Kind of CI task is not appear general development, feels like rare setup React repo specific.
Anyway I'm going to find what does job do tracking git history. 👨‍💻

@bvaughn
Copy link
Contributor

bvaughn commented Aug 22, 2021

Running yarn replace-fork on this branch results in a bunch of bogus replacements: changing import paths in several .old.js files to reference .new.js modules. This does not happen on HEAD of main branch though.

I rebased your branch on main and tried re-running and it still fails. Weird.

@acdlite
Copy link
Collaborator

acdlite commented Aug 22, 2021

It's caused by an ESLint error causing the replace-fork script to silently crash. (The replace-fork script relies on ESLint to fix the imports.)

Try running yarn linc on this branch and you'll see the error:

Error: Failed to load config "react-app" to extend from.
Referenced from: fixtures/blocks/package.json

@bvaughn
Copy link
Contributor

bvaughn commented Aug 22, 2021

Now that #22156 has landed, if you rebase on HEAD of main branch you'll see the underlying error here in CI (or when you run yarn replace-fork)

@ryota-murakami
Copy link
Contributor Author

After rebase and push for apply this fix,

Still failed but showing ESLint message Error: Failed to load config "react-app" to extend from. with exit code 1.
It works! 😄

@stale
Copy link

stale bot commented Jan 8, 2022

This pull request has been automatically marked as stale. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated.

@stale stale bot added the Resolution: Stale Automatically closed due to inactivity label Jan 8, 2022
Copy link

Closing this pull request after a prolonged period of inactivity. If this issue is still present in the latest release, please ask for this pull request to be reopened. Thank you!

@github-actions github-actions bot closed this Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Resolution: Stale Automatically closed due to inactivity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants