chore: update ink to v3 #26190
chore: update ink to v3 #26190
Conversation
gatsbot
bot
added
the
status: triage needed
vladar
added
status: needs core review
|
I converted the PR to draft until it's ready. Thanks for doing this upgrade! 💜 |
|
Still looking into this duplicate message at the end; I've created a simplified
Outputs the following:
It seems consistently after the 26th index it starts repeating the 26th, 27th. 28th, etc messages. |
|
@herecydev Thanks for picking this up! |
|
No problem, I'm very close to getting an understanding of the duplicate log (tracking that in main ink repo). Ink-spinner still needs fixing as well; otherwise this shouldn't be too far away |
|
Looking much healthier now: Build Dev Dev (forced error) Recipes |
|
@sidharthachatterjee hopefully you can help with this one. Trying to fix up the tests and getting stuck on this line: https://github.com/gatsbyjs/gatsby/blob/master/integration-tests/gatsby-cli/__tests__/new.js#L11 I'm not sure if this is a script that should have been resolved, but I'm getting:
|
|
It looks like the integration test is trying to call - const clean = dir => execa(`yarn`, ["del-cli", dir])`
+ const clean = dir => execa(`yarn`, ["del", dir])`Hopefully this helps :) |
|
I'm on windows and it explicity calls out using
|
|
Gotcha. I cloned your PR just now on Windows and WSL and the |
|
I'll give it another try then 🤔 the errors look fairly straightforward to fix regardless |
fix: update bundled ink
fix: update caniuse-lite so tests pass
|
This should be ready @wardpeet. I don't have direct write access to this PR, so I can't fix the 2 conflicts. |
I'll fix the conflicts and do some more testing ;) |
So interesting thing I discovered as I worked on resolving conflicts on this PR + getting it through finish line. https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby-cli/src/reporter/start-logger.ts#L12-L16 checks if After fixing above check (well removing it, because it's not needed anymore) I also did hit this problem and this is because So rollup (well, our setup at least) doesn't "prefer" default so instead of I also want to sure couple more niceties that this upgrade - ink@2 (or maybe the way we use it now) sometimes truncates longer messages ( #19658 ) - this PR (so this might be either ink upgrade or component changes done here or both) actually seems to fix that - see https://www.loom.com/share/925e12ce153c49d2a0ddb7d1ae7bb1a6
|
…... because `ink` module wasn't installed - it was bundled in `gatsby-cli` (yikes)
… accumulate output as is and instead of checking exact output, replace spaces with any whitespace in matcher
| @@ -17,7 +17,7 @@ export const StoreStateProvider: React.FC = ({ | |||
| (getStore().getState() as any) as IGatsbyState | |||
| ) | |||
|
|
|||
| useEffect( | |||
| useLayoutEffect( | |||
There was a problem hiding this comment.
This seems needed for quick commands (like gatsby options) that are in practice short sync ink@3 doesn't seem to run useEffect before we start printing things and result in no output in those cases
|
I published |
|
Ok, it seems like I did broke |
|
New version were published for |
* Fix most stuff * Whitespace * Upgrade spinners * Format * Wrap error in text * Fix devDeps * Remove ink-box * Eslint fix * Eslint * Eslint * Eslint * Fix return type missing * fix: update bundled ink * fix: update caniuse-lite so tests pass * since we started bundling ink resolution check was almost never true ... because `ink` module wasn't installed - it was bundled in `gatsby-cli` (yikes) * workaround rollup problem when trying to import cjs with default * some tests should be fixed * ink auto wraps output, so current ln splitting won't work well - just accumulate output as is and instead of checking exact output, replace spaces with any whitespace in matcher * fix "default" import thingies * add comment about nullable stdout Co-authored-by: Kyle Mathews <mathews.kyle@gmail.com> Co-authored-by: Adam Schay <adamschay@gmail.com> Co-authored-by: Michal Piechowiak <misiek.piechowiak@gmail.com> (cherry picked from commit 1e702ae)
* Fix most stuff * Whitespace * Upgrade spinners * Format * Wrap error in text * Fix devDeps * Remove ink-box * Eslint fix * Eslint * Eslint * Eslint * Fix return type missing * fix: update bundled ink * fix: update caniuse-lite so tests pass * since we started bundling ink resolution check was almost never true ... because `ink` module wasn't installed - it was bundled in `gatsby-cli` (yikes) * workaround rollup problem when trying to import cjs with default * some tests should be fixed * ink auto wraps output, so current ln splitting won't work well - just accumulate output as is and instead of checking exact output, replace spaces with any whitespace in matcher * fix "default" import thingies * add comment about nullable stdout Co-authored-by: Kyle Mathews <mathews.kyle@gmail.com> Co-authored-by: Adam Schay <adamschay@gmail.com> Co-authored-by: Michal Piechowiak <misiek.piechowiak@gmail.com> (cherry picked from commit 1e702ae) Co-authored-by: Dan Kirkham <herecy@live.co.uk>
* Fix most stuff * Whitespace * Upgrade spinners * Format * Wrap error in text * Fix devDeps * Remove ink-box * Eslint fix * Eslint * Eslint * Eslint * Fix return type missing * fix: update bundled ink * fix: update caniuse-lite so tests pass * since we started bundling ink resolution check was almost never true ... because `ink` module wasn't installed - it was bundled in `gatsby-cli` (yikes) * workaround rollup problem when trying to import cjs with default * some tests should be fixed * ink auto wraps output, so current ln splitting won't work well - just accumulate output as is and instead of checking exact output, replace spaces with any whitespace in matcher * fix "default" import thingies * add comment about nullable stdout Co-authored-by: Kyle Mathews <mathews.kyle@gmail.com> Co-authored-by: Adam Schay <adamschay@gmail.com> Co-authored-by: Michal Piechowiak <misiek.piechowiak@gmail.com>
Description
Updates ink to v3. Pretty straightforward migration for most things; but some small bugs to fix:
Closes #19658