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: use node-fetch only as fallback #2688

Merged
merged 1 commit into from
Sep 19, 2023
Merged

fix: use node-fetch only as fallback #2688

merged 1 commit into from
Sep 19, 2023

Conversation

tido64
Copy link
Member

@tido64 tido64 commented Sep 18, 2023

Description

Use node-fetch only as fallback when current Node version doesn't implement Fetch API

Test plan

Repeat the following steps in both Node 16 and Node 18:

cd packages/test-app
yarn build --dependencies
yarn start

Once the dev server is running, press j. A message should indicate that we're trying to open the debugger, but nothing will show up. Just verify that it doesn't crash.

In a separate terminal, try to start another dev server:

% yarn start
error Another process is using port 8081. Please terminate this process and try again, or try another port with `--port`.

This one should fail with the error message above.

On Node 16, this is an error message we're trying to avoid:

node:internal/readline/emitKeypressEvents:71
            throw err;
            ^

ReferenceError: fetch is not defined
    at ReadStream.<anonymous> (/Users/tido/Source/rnx-kit/packages/cli/lib/serve/keyboard.js:36:21)
    at ReadStream.emit (node:events:513:28)
    at emitKeys (node:internal/readline/utils:357:14)
    at emitKeys.next (<anonymous>)
    at ReadStream.onData (node:internal/readline/emitKeypressEvents:61:36)
    at ReadStream.emit (node:events:513:28)
    at addChunk (node:internal/streams/readable:315:12)
    at readableAddChunk (node:internal/streams/readable:289:9)
    at ReadStream.Readable.push (node:internal/streams/readable:228:10)
    at TTY.onStreamRead (node:internal/stream_base_commons:190:23)

@github-actions github-actions bot added feature: metro This is related to Metro feature: cli This is related to CLI labels Sep 18, 2023
Copy link
Contributor

@kelset kelset left a comment

Choose a reason for hiding this comment

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

works as expected, a pity we need to still support Node 16 in 2023 when LTS is 18 🙈

@tido64 tido64 merged commit 8ba65d6 into main Sep 19, 2023
12 checks passed
@tido64 tido64 deleted the tido/fetch branch September 19, 2023 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: cli This is related to CLI feature: metro This is related to Metro
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants