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

Upgrade xo #130

Merged
merged 2 commits into from
Jul 21, 2024
Merged

Upgrade xo #130

merged 2 commits into from
Jul 21, 2024

Conversation

ehmicky
Copy link
Collaborator

@ehmicky ehmicky commented Jul 21, 2024

This PR upgrades xo.

CI currently failing due to actions/node-versions#182

@@ -46,6 +46,7 @@ const resizeArrayBuffer = (contents, length) => {
return contents;
}

// eslint-disable-next-line n/no-unsupported-features/es-syntax
const arrayBuffer = new ArrayBuffer(length, {maxByteLength: getNewContentsLength(length)});
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ArrayBuffer resizing is not supported in Node 18.
However, we check for support before calling this function.

@@ -8,6 +8,7 @@ export class MaxBufferError extends Error {

// eslint-disable-next-line @typescript-eslint/ban-types
type TextStreamItem = string | Buffer | ArrayBuffer | ArrayBufferView;
// eslint-disable-next-line n/no-unsupported-features/node-builtins
export type AnyStream<SteamItem = TextStreamItem> = Readable | ReadableStream<SteamItem> | AsyncIterable<SteamItem>;
Copy link
Collaborator Author

@ehmicky ehmicky Jul 21, 2024

Choose a reason for hiding this comment

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

ReadableStream is supported in Node 18 but is marked as experimental.
However, it is a web standard, and the Node.js implementation has been pretty stable for a few years now.
This is only used in types, not runtime.

@@ -5,6 +5,7 @@ import {finished} from 'node:stream/promises';
export {fromAnyIterable as readableStreamFrom} from '@sec-ant/readable-stream/ponyfill';

export const createStream = streamDefinition => typeof streamDefinition === 'function'
// eslint-disable-next-line n/no-unsupported-features/node-builtins
? Duplex.from(streamDefinition)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Duplex.from() is supported in Node 18 but is marked as experimental.
However, its implementation has barely changed in many years now. Also, this is only used in tests.

@@ -63,13 +64,15 @@ if (!nodeVersion.startsWith('v16.')) {
test('works with readableWebStream({ type: "bytes" })', readableWebStream, 'bytes');

test('works with fetch()', async t => {
// eslint-disable-next-line n/no-unsupported-features/node-builtins
const {body} = await fetch(TEST_URL);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fetch() support is experimental in Node, but we're only using this in tests.

const {body} = await fetch(TEST_URL);
const result = await getStream(body);
const parsedResult = JSON.parse(result);
t.true(Array.isArray(parsedResult));
});

test('can use TextDecoderStream', async t => {
// eslint-disable-next-line n/no-unsupported-features/node-builtins
const textDecoderStream = new TextDecoderStream('utf-16le');
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same comment as for fetch(), but with TextDecoderStream().

@@ -274,6 +274,7 @@ const testMultipleReads = async (t, wait) => {
assertSuccess(t, stream, Readable);
};

// eslint-disable-next-line n/no-unsupported-features/node-builtins
test('Handles multiple successive fast reads', testMultipleReads, () => scheduler.yield());
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same comment as fetch(), but for scheduler.yield().

@sindresorhus sindresorhus merged commit cd27167 into main Jul 21, 2024
2 of 4 checks passed
@sindresorhus sindresorhus deleted the upgrade-deps branch July 21, 2024 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants