Migrate Ink to ESM and update React #546

pepicrft · 2023-01-30T17:36:56Z

Resolves #543

I'm migrating this package to ESM to nudge the ecosystem towards embracing the standards.

Update Typescript
Update @sindresorhus/tsconfig
- Fix some type-checking issues after the update
Set exports attribute in the package.json.
Set moduleResolution and module attributes in the tsconfig.json to node16
Update some dependencies to use their ESM version instead of relying on CJS/ESM interoperability.

pepicrft · 2023-01-30T18:00:15Z

package.json

-		],
-		"require": [
-			"ts-node/register/transpile-only"
+		"extensions": {


I'm working on updating the tests to ESM and fixing them.

pepicrft · 2023-02-03T12:09:26Z

@sindresorhus I might need your help to get the tests passing. For some reason when I run the tests they time out without any useful output. My guess is that the ts-node node module loader fails or hangs internally and AVA ends up timing out. I'd appreciate if you could give me a hand or pointers with this one.

sindresorhus · 2023-02-03T13:31:34Z

Try this config: https://github.com/sindresorhus/got/blob/0ca0b7f7134f41b45a51370154041cc97c28ca60/tsconfig.json#L17-L21

pepicrft · 2023-02-03T14:00:37Z

That tricked worked 🚀. Thanks a lot @sindresorhus. I'll work on fixing the tests and I'll let you know when the PR is ready for a review.

vadimdemedes · 2023-02-04T08:06:41Z

Thanks for this massive contribution, @pepicrft! Let me know when it's ready to review 👍

vadimdemedes · 2023-02-04T08:16:21Z

You can change CI to only test Node.js 14, 16 and 18, since 10 and 12 aren't maintained anymore. This will allow tests to run in this PR.

pepicrft · 2023-02-06T14:14:00Z

benchmark/static/static.tsx

@@ -1,7 +1,9 @@
+#!/usr/bin/env npx ts-node-esm


I changed the approach here. Instead of using import-jsx in an index.js file I added a shebang at the top to run the script through ts-node-esm, which will use Node loaders to transform the module into standard JS.

test/hooks.tsx

pepicrft · 2023-02-06T14:35:52Z

test/errors.tsx

 });

 test.after(() => {
 	restore();
 });

-test('catch and display error', t => {
+test.skip('catch and display error', t => {


@vadimdemedes the output changed and it's causing the test to fail. Where is the formatting of the error happening?

Errors are caught in App component (src/components/App.tsx) and formatted inside ErrorOverview component (src/components/ErrorOverview.tsx).

pepicrft · 2023-02-06T14:41:00Z

Quick update here @vadimdemedes & @sindresorhus:

The CI pipeline has been adjusted to run with Node 14, 16, and 18
I updated xo and tackled some linting that showed up due to changes in rules.
Went through all the tests and did the necessary adjustments to make them pass.
Updated React to 18

There are some remaining bits but I'm a bit blocked with them:

I added skip to a test that's testing that the right contend is being logged through the standard output. I can't figure out where the formatting is happening and why it broke with the code changes.
I disabled the test modules that use node-pty. As mentioned here, Node crashes when the tests try to interact with the pseudo-terminal instance.

I'd appreciate your help to get this one to the finish line 🙏🏼

vadimdemedes · 2023-02-08T17:56:34Z

Not sure what's happening yet, but running npx xo fails immediately with this error:

❯ npx xo
node:internal/process/esm_loader:94
    internalBinding('errors').triggerUncaughtException(
                              ^

Error [ERR_MODULE_NOT_FOUND]: Cannot find package '/Users/vadimdemedes/Projects/ink/node_modules/xo/node_modules/eslint/' imported from /Users/vadimdemedes/Projects/ink/node_modules/xo/index.js
Did you mean to import eslint/lib/api.js?
    at new NodeError (node:internal/errors:372:5)
    at legacyMainResolve (node:internal/modules/esm/resolve:341:9)
    at packageResolve (node:internal/modules/esm/resolve:941:14)
    at moduleResolve (node:internal/modules/esm/resolve:1003:20)
    at defaultResolve (node:internal/modules/esm/resolve:1218:11)
    at ESMLoader.resolve (node:internal/modules/esm/loader:580:30)
    at ESMLoader.getModuleJob (node:internal/modules/esm/loader:294:18)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:80:40)
    at link (node:internal/modules/esm/module_job:78:36) {
  code: 'ERR_MODULE_NOT_FOUND'
}

Have you encountered this before? I deleted node_modules and reinstalled everything.

vadimdemedes · 2023-02-08T18:15:32Z

Same error is actually happening on CI. Interestingly, XO fails differently on Node.js 14 and 18 on CI.

src/components/Static.tsx

vadimdemedes · 2023-02-23T19:55:45Z

I also reverted your changes to make the test pass. For some reasons I still don't know they are no longer necessary. I still have the feeling is flakiness but I can't figure out why.

@pepicrft Looks like it's still failing on CI without this fix that got reverted.

pepicrft · 2023-02-23T20:27:14Z

@pepicrft Looks like it's still failing on CI without this fix that got reverted.

Ouch :/. It is possible then that the test's implementation depends on the environment in which it runs.

vadimdemedes · 2023-02-25T11:21:50Z

Oh my, after several days of debugging, I finally fixed the useStdout test in test/hooks.tsx and CI is green again.

For context, Ink has 3 rendering modes:

Default mode - renders as soon as possible, throttled to 32ms.
CI mode - collects all render frames (cycles) and writes them to stdout once after app is unmounted. This is necessary, because CI services normally don't support ANSI escapes that clear lines.
Debug mode - renders immediately without throttling and without clearing previous output. Basically console.log.

When I ran this PR's tests locally, useStdout test has succeeded. On GitHub Actions it was always failing. Expected output from test/fixtures/use-stdout.tsx script is:

Hello world
Hello from Ink to stdout
Hello world
exited

But the first "Hello world" was never displayed on CI.

I started adding a bunch of console.logs to tests and Ink's codebase and I noticed something odd. isCi was still true when running test/fixtures/use-stdout.tsx. Despite the fact that Ink's tests explicitly delete CI environment variable → https://github.com/pepicrft/ink/blob/2238385acff19b6b12159c40dd702da24ce264f0/test/hooks.tsx#L31.

Since it worked before this PR, there must've been some change in how CI enivronment is detected. I traced it down to this commit in ci-info package, which is a dependency of is-ci that Ink uses → watson/ci-info@24cc450.

Looks like deleting CI environment isn't enough anymore and an explicit CI=false must be set to disable CI detection. For context, we want to make Ink's tests think they're not running in CI, so that we can replicate behavior of Ink in real-world usage.

So here's the fix → pepicrft@67d7a8a.

vadimdemedes · 2023-02-25T11:24:04Z

The last thing to figure out before shipping this PR is a replacement of import-jsx, since it's not supported in ESM anymore. It seems to be quite popular, so we need to have either a replacement or some recommendations in the migration guide.

vadimdemedes · 2023-02-27T21:46:14Z

Submitted vadimdemedes/import-jsx#18 to support ESM in import-jsx. I also realized that packages like ink-text-input also need to be converted to ESM. Hopefully we can ship it all this week.

pepicrft · 2023-02-28T11:25:56Z

Just WOW on that thorough debugging. I wouldn't have been able to figure that out myself.
Regarding the updating of import-jsx, how do you feel about using an existing Node loader and deprecating that package? It'll be less maintenance work for you going forward and we can focus the efforts on Ink. Is there any Ink component other than ink-text-input that we should also update? I'm happy to take a look at that one.

vadimdemedes · 2023-02-28T19:58:42Z

Regarding the updating of import-jsx, how do you feel about using an existing Node loader and deprecating that package? It'll be less maintenance work for you going forward and we can focus the efforts on Ink.

Replied in vadimdemedes/import-jsx#18 (comment).

Is there any Ink component other than ink-text-input that we should also update? I'm happy to take a look at that one.

Yep, here are the packages I wanted to update too:

Going to start working on these too and report back.

vadimdemedes · 2023-02-28T19:59:01Z

Thanks for an amazing contribution, @pepicrft 🚢

pepicrft · 2023-02-28T20:21:54Z

Thanks a lot @vadimdemedes and @sindresorhus for supporting through this effort. It's a pleasure contributing to open source projects like Ink.

pepicrft · 2023-03-01T13:03:35Z

@vadimdemedes when is the next release going out?

vadimdemedes · 2023-03-01T18:36:55Z

Ideally I want to have the other projects I linked to above converted to ESM too, so that all of them are released roughly at the same time.

pepicrft added 3 commits January 30, 2023 18:17

Update @sindresorhus/tsconfig

c689da6

Update Typescript

0898f7b

Fix imports

5874525

pepicrft mentioned this pull request Jan 30, 2023

Update React to 18 #545

Closed

pepicrft added 2 commits January 30, 2023 18:56

Update some dependencies

1e9ec9d

Fix some tests

ee66505

pepicrft changed the title ~~Migrate Ink to ESM~~ [WIP] Migrate Ink to ESM Jan 30, 2023

pepicrft commented Jan 30, 2023

View reviewed changes

Fix typescript issues in tests

2792a1d

Only run it on CI with Node versions 14, 16, and 18

4c81ae8

pepicrft changed the title ~~[WIP] Migrate Ink to ESM~~ Migrate Ink to ESM and update React Feb 6, 2023

pepicrft added 4 commits February 6, 2023 14:25

Fix some tests

a03ab43

Update xo and fix issues

a0a02ca

Remove .tool-verions

48812a7

Fix benchmark scripts

1b8f7c9

pepicrft commented Feb 6, 2023

View reviewed changes

Delete import-jsx

c0f6bea

pepicrft commented Feb 6, 2023

View reviewed changes

test/hooks.tsx Show resolved Hide resolved

pepicrft commented Feb 6, 2023

View reviewed changes

sindresorhus and others added 2 commits February 9, 2023 01:20

Update package.json

bdb6c51

Fix test for error formatting

bc7dfc5

sindresorhus reviewed Feb 8, 2023

View reviewed changes

src/components/Static.tsx Outdated Show resolved Hide resolved

vadimdemedes added 3 commits February 21, 2023 22:06

Minor tweaks to examples

fd4ca63

Minor tweaks to lint config

9218dfa

Minor tweaks to benchmarks

7125210

Test tweaks

2238385

vadimdemedes added 3 commits February 23, 2023 22:33

Minor tweaks to examples

344bb2b

Force disabling of CI detection in tests

67d7a8a

Minor tweaks and fixes

2c4a81b

vadimdemedes added 4 commits February 28, 2023 20:30

Fix Devtools

fe33ea4

Update readme

80c31ca

Minor tweaks

07de311

Mention @esbuild-kit/esm-loader in readme

f66021d

vadimdemedes mentioned this pull request Feb 28, 2023

Use loader hooks vadimdemedes/import-jsx#18

Merged

vadimdemedes merged commit aaf45a1 into vadimdemedes:master Feb 28, 2023

This was referenced Mar 1, 2023

Replace yoga-layout-prebuilt with yoga-wasm-web #550

Merged

Migrate to ESM and Ink 4 vadimdemedes/ink-testing-library#19

Merged

Migrate to ESM and Ink 4 vadimdemedes/ink-spinner#16

Merged

This was referenced Mar 1, 2023

Migrate to ESM and Ink 4 vadimdemedes/ink-text-input#76

Merged

Migrate to ESM and Ink 4 vadimdemedes/ink-select-input#46

Merged

vadimdemedes mentioned this pull request Mar 17, 2023

Use the same rendering approach on CI and non-CI environment by default #266

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate Ink to ESM and update React #546

Migrate Ink to ESM and update React #546

pepicrft commented Jan 30, 2023

pepicrft Jan 30, 2023

pepicrft commented Feb 3, 2023

sindresorhus commented Feb 3, 2023

pepicrft commented Feb 3, 2023

vadimdemedes commented Feb 4, 2023

vadimdemedes commented Feb 4, 2023

pepicrft Feb 6, 2023

pepicrft Feb 6, 2023

vadimdemedes Feb 8, 2023

pepicrft commented Feb 6, 2023

vadimdemedes commented Feb 8, 2023

vadimdemedes commented Feb 8, 2023 •

edited

vadimdemedes commented Feb 23, 2023

pepicrft commented Feb 23, 2023

vadimdemedes commented Feb 25, 2023

vadimdemedes commented Feb 25, 2023

vadimdemedes commented Feb 27, 2023

pepicrft commented Feb 28, 2023

vadimdemedes commented Feb 28, 2023

vadimdemedes commented Feb 28, 2023

pepicrft commented Feb 28, 2023

pepicrft commented Mar 1, 2023

vadimdemedes commented Mar 1, 2023

Migrate Ink to ESM and update React #546

Migrate Ink to ESM and update React #546

Conversation

pepicrft commented Jan 30, 2023

pepicrft Jan 30, 2023

Choose a reason for hiding this comment

pepicrft commented Feb 3, 2023

sindresorhus commented Feb 3, 2023

pepicrft commented Feb 3, 2023

vadimdemedes commented Feb 4, 2023

vadimdemedes commented Feb 4, 2023

pepicrft Feb 6, 2023

Choose a reason for hiding this comment

pepicrft Feb 6, 2023

Choose a reason for hiding this comment

vadimdemedes Feb 8, 2023

Choose a reason for hiding this comment

pepicrft commented Feb 6, 2023

vadimdemedes commented Feb 8, 2023

vadimdemedes commented Feb 8, 2023 • edited

vadimdemedes commented Feb 23, 2023

pepicrft commented Feb 23, 2023

vadimdemedes commented Feb 25, 2023

vadimdemedes commented Feb 25, 2023

vadimdemedes commented Feb 27, 2023

pepicrft commented Feb 28, 2023

vadimdemedes commented Feb 28, 2023

vadimdemedes commented Feb 28, 2023

pepicrft commented Feb 28, 2023

pepicrft commented Mar 1, 2023

vadimdemedes commented Mar 1, 2023

vadimdemedes commented Feb 8, 2023 •

edited