Skip to content

Commit

Permalink
ARROW-12704: [JS] Support and use optional chaining
Browse files Browse the repository at this point in the history 
Optional chaining makes the code more concise and easier to reason about.

Needs a fork of esm because of standard-things/esm#866.

Closes apache#10278 from domoritz/modern-js

Lead-authored-by: Dominik Moritz <domoritz@gmail.com>
Co-authored-by: p42-ai[bot] <72252241+p42-ai[bot]@users.noreply.github.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
  • Loading branch information
@domoritz @p42-ai @michalursa
2 people authored and michalursa committed Jun 13, 2021
1 parent 300e985 commit 951f476
Show file tree
Hide file tree
Showing 21 changed files with 92 additions and 99 deletions.
8 changes: 4 additions & 4 deletions js/bin/integration.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ const exists = async (p) => {
}
})()
.then((x) => +x || 0, (e) => {
e && process.stderr.write(`${e && e.stack || e}\n`);
e && process.stderr.write(`${e?.stack || e}\n`);
return process.exitCode || 1;
}).then((code) => process.exit(code));

Expand Down Expand Up @@ -141,7 +141,7 @@ function validateReaderIntegration(jsonData, arrowBuffer) {
for (const [jsonRecordBatch, binaryRecordBatch] of zip(jsonReader, binaryReader)) {
compareTableIsh(jsonRecordBatch, binaryRecordBatch);
}
} catch (e) { throw new Error(`${msg}: fail \n ${e && e.stack || e}`); }
} catch (e) { throw new Error(`${msg}: fail \n ${e?.stack || e}`); }
process.stdout.write(`${msg}: pass\n`);
}

Expand All @@ -151,7 +151,7 @@ function validateTableFromBuffersIntegration(jsonData, arrowBuffer) {
const jsonTable = Table.from(jsonData);
const binaryTable = Table.from(arrowBuffer);
compareTableIsh(jsonTable, binaryTable);
} catch (e) { throw new Error(`${msg}: fail \n ${e && e.stack || e}`); }
} catch (e) { throw new Error(`${msg}: fail \n ${e?.stack || e}`); }
process.stdout.write(`${msg}: pass\n`);
}

Expand All @@ -164,7 +164,7 @@ function validateTableToBuffersIntegration(srcFormat, arrowFormat) {
const srcTable = Table.from(srcFormat === `json` ? jsonData : arrowBuffer);
const dstTable = Table.from(srcTable.serialize(`binary`, arrowFormat === `stream`));
compareTableIsh(dstTable, refTable);
} catch (e) { throw new Error(`${msg}: fail \n ${e && e.stack || e}`); }
} catch (e) { throw new Error(`${msg}: fail \n ${e?.stack || e}`); }
process.stdout.write(`${msg}: pass\n`);
};
}
Expand Down
2 changes: 1 addition & 1 deletion js/gulp/closure-task.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ const closureTask = ((cache) => memoizeTask(cache, async function closure(target
const exportedImports = publicModulePaths(srcAbsolute).reduce((entries, publicModulePath) => [
...entries, {
publicModulePath,
exports_: getPublicExportedNames(esmRequire(publicModulePath, { warnings: false }))
exports_: getPublicExportedNames(esmRequire(publicModulePath))
}
], []);

Expand Down
3 changes: 1 addition & 2 deletions js/gulp/minify-task.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ const {
targetDir,
mainExport,
UMDSourceTargets,
terserLanguageNames,
shouldRunInChildProcess,
spawnGulpCommandInChildProcess,
} = require('./util');
Expand Down Expand Up @@ -64,7 +63,7 @@ const minifyTask = ((cache, commonConfig) => memoizeTask(cache, function minifyJ
minimizer: [
new TerserPlugin({
terserOptions: {
ecma: terserLanguageNames[target],
ecma: target,
output: { comments: false },
compress: { unsafe: true }
},
Expand Down
1 change: 0 additions & 1 deletion js/gulp/test-task.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ const ARROW_JAVA_DIR = process.env.ARROW_JAVA_DIR || path.join(ARROW_HOME, 'java
const CPP_EXE_PATH = process.env.ARROW_CPP_EXE_PATH || path.join(ARROW_HOME, 'cpp/build/debug');
const ARROW_INTEGRATION_DIR = process.env.ARROW_INTEGRATION_DIR || path.join(ARROW_HOME, 'integration');
const CPP_JSON_TO_ARROW = path.join(CPP_EXE_PATH, 'arrow-json-integration-test');
const CPP_STREAM_TO_FILE = path.join(CPP_EXE_PATH, 'arrow-stream-to-file');
const CPP_FILE_TO_STREAM = path.join(CPP_EXE_PATH, 'arrow-file-to-stream');

const testFilesDir = path.join(ARROW_HOME, 'js/test/data');
Expand Down
10 changes: 3 additions & 7 deletions js/gulp/util.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ const gCCLanguageNames = {
es2015: `ECMASCRIPT_2015`,
es2016: `ECMASCRIPT_2016`,
es2017: `ECMASCRIPT_2017`,
es2018: `ECMASCRIPT_2018`,
es2019: `ECMASCRIPT_2019`,
esnext: `ECMASCRIPT_NEXT`
};

Expand All @@ -72,12 +74,6 @@ const UMDSourceTargets = {
esnext: `esnext`
};

const terserLanguageNames = {
es5: 5, es2015: 6,
es2016: 7, es2017: 8,
esnext: 8 // <--- ?
};

// ES7+ keywords Terser shouldn't mangle
// Hardcoded here since some are from ES7+, others are
// only defined in interfaces, so difficult to get by reflection.
Expand Down Expand Up @@ -211,7 +207,7 @@ module.exports = {
mainExport, npmPkgName, npmOrgName, metadataFiles, packageJSONFields,

knownTargets, knownModules, tasksToSkipPerTargetOrFormat,
gCCLanguageNames, UMDSourceTargets, terserLanguageNames,
gCCLanguageNames, UMDSourceTargets,

taskName, packageName, tsconfigName, targetDir, combinations, observableFromStreams,
ESKeywords, publicModulePaths, esmRequire, shouldRunInChildProcess, spawnGulpCommandInChildProcess
Expand Down
4 changes: 2 additions & 2 deletions js/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,9 @@
"del-cli": "3.0.1",
"eslint": "^7.24.0",
"eslint-plugin-jest": "^24.3.5",
"esm": "3.2.25",
"esm": "https://github.com/jsg2021/esm/releases/download/v3.x.x-pr883/esm-3.x.x-pr883.tgz",
"glob": "7.1.4",
"google-closure-compiler": "20210406.0.0",
"google-closure-compiler": "20210505.0.0",
"gulp": "4.0.2",
"gulp-json-transform": "0.4.7",
"gulp-rename": "2.0.0",
Expand Down
4 changes: 2 additions & 2 deletions js/src/bin/arrow2csv.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ type ToStringState = {
})()
.then((x) => +x || 0, (err) => {
if (err) {
console.error(`${err && err.stack || err}`);
console.error(`${err?.stack || err}`);
}
return process.exitCode || 1;
}).then((code) => process.exit(code));
Expand Down Expand Up @@ -147,7 +147,7 @@ function batchesToString(state: ToStringState, schema: Schema) {
},
transform(batch: RecordBatch, _enc: string, cb: (error?: Error, data?: any) => void) {

batch = !(state.schema && state.schema.length) ? batch : batch.select(...state.schema);
batch = !state.schema?.length ? batch : batch.select(...state.schema);

if (state.closed) { return cb(undefined, null); }

Expand Down
8 changes: 4 additions & 4 deletions js/src/io/node/iterable.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class IterableReadable<T extends Uint8Array | any> extends Readable {
const it = this._iterator;
let fn: any;
it && (fn = e != null && it.throw || it.return);
fn && fn.call(it, e);
fn?.call(it, e);
cb && cb(null);
}
private _pull(size: number, it: SourceIterator<T>) {
Expand All @@ -66,7 +66,7 @@ class IterableReadable<T extends Uint8Array | any> extends Readable {
}
if (!this.push(r.value) || size <= 0) { break; }
}
if ((r && r.done || !this.readable) && (this.push(null) || true)) {
if ((r?.done || !this.readable) && (this.push(null) || true)) {
it.return && it.return();
}
return !this.readable;
Expand Down Expand Up @@ -94,7 +94,7 @@ class AsyncIterableReadable<T extends Uint8Array | any> extends Readable {
const it = this._iterator;
let fn: any;
it && (fn = e != null && it.throw || it.return);
fn && fn.call(it, e).then(() => cb && cb(null)) || (cb && cb(null));
fn?.call(it, e).then(() => cb && cb(null)) || (cb && cb(null));
}
private async _pull(size: number, it: AsyncSourceIterator<T>) {
const bm = this._bytesMode;
Expand All @@ -105,7 +105,7 @@ class AsyncIterableReadable<T extends Uint8Array | any> extends Readable {
}
if (!this.push(r.value) || size <= 0) { break; }
}
if ((r && r.done || !this.readable) && (this.push(null) || true)) {
if ((r?.done || !this.readable) && (this.push(null) || true)) {
it.return && it.return();
}
return !this.readable;
Expand Down
6 changes: 3 additions & 3 deletions js/src/io/node/reader.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,12 @@ class RecordBatchReaderDuplex<T extends { [key: string]: DataType } = any> exten
}
_final(cb?: CB) {
const aq = this._asyncQueue;
aq && aq.close();
aq?.close();
cb && cb();
}
_write(x: any, _: string, cb: CB) {
const aq = this._asyncQueue;
aq && aq.write(x);
aq?.write(x);
cb && cb();
return true;
}
Expand Down Expand Up @@ -77,7 +77,7 @@ class RecordBatchReaderDuplex<T extends { [key: string]: DataType } = any> exten
while (this.readable && !(r = await reader.next()).done) {
if (!this.push(r.value) || (size != null && --size <= 0)) { break; }
}
if (!this.readable || (r && r.done && (reader.autoDestroy || (await reader.reset().open()).closed))) {
if (!this.readable || (r?.done && (reader.autoDestroy || (await reader.reset().open()).closed))) {
this.push(null);
await reader.cancel();
}
Expand Down
6 changes: 3 additions & 3 deletions js/src/io/node/writer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,12 @@ class RecordBatchWriterDuplex<T extends { [key: string]: DataType } = any> exten
}
_final(cb?: CB) {
const writer = this._writer;
writer && writer.close();
writer?.close();
cb && cb();
}
_write(x: any, _: string, cb: CB) {
const writer = this._writer;
writer && writer.write(x);
writer?.write(x);
cb && cb();
return true;
}
Expand All @@ -68,7 +68,7 @@ class RecordBatchWriterDuplex<T extends { [key: string]: DataType } = any> exten
}
if (!this.push(r.value) || size <= 0) { break; }
}
if ((r && r.done || !this.readable)) {
if ((r?.done || !this.readable)) {
this.push(null);
await reader.cancel();
}
Expand Down
12 changes: 6 additions & 6 deletions js/src/io/whatwg/iterable.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,14 @@ export function toDOMStream<T>(source: Iterable<T> | AsyncIterable<T>, options?:
function iterableAsReadableDOMStream<T>(source: Iterable<T>, options?: ReadableDOMStreamOptions) {

let it: SourceIterator<T> | null = null;
const bm = (options && options.type === 'bytes') || false;
const hwm = options && options.highWaterMark || (2 ** 24);
const bm = (options?.type === 'bytes') || false;
const hwm = options?.highWaterMark || (2 ** 24);

return new ReadableStream<T>({
...options as any,
start(controller) { next(controller, it || (it = source[Symbol.iterator]() as SourceIterator<T>)); },
pull(controller) { it ? (next(controller, it)) : controller.close(); },
cancel() { (it && (it.return && it.return()) || true) && (it = null); }
cancel() { (it?.return && it.return() || true) && (it = null); }
}, { highWaterMark: bm ? hwm : undefined, ...options });

function next(controller: ReadableStreamDefaultController<T>, it: SourceIterator<T>) {
Expand All @@ -66,14 +66,14 @@ function iterableAsReadableDOMStream<T>(source: Iterable<T>, options?: ReadableD
function asyncIterableAsReadableDOMStream<T>(source: AsyncIterable<T>, options?: ReadableDOMStreamOptions) {

let it: AsyncSourceIterator<T> | null = null;
const bm = (options && options.type === 'bytes') || false;
const hwm = options && options.highWaterMark || (2 ** 24);
const bm = (options?.type === 'bytes') || false;
const hwm = options?.highWaterMark || (2 ** 24);

return new ReadableStream<T>({
...options as any,
async start(controller) { await next(controller, it || (it = source[Symbol.asyncIterator]() as AsyncSourceIterator<T>)); },
async pull(controller) { it ? (await next(controller, it)) : controller.close(); },
async cancel() { (it && (it.return && await it.return()) || true) && (it = null); },
async cancel() { (it?.return && await it.return() || true) && (it = null); },
}, { highWaterMark: bm ? hwm : undefined, ...options });

async function next(controller: ReadableStreamDefaultController<T>, it: AsyncSourceIterator<T>) {
Expand Down
10 changes: 5 additions & 5 deletions js/src/ipc/message.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export class MessageReader implements IterableIterator<Message> {
public readSchema(throwIfNull = false) {
const type = MessageHeader.Schema;
const message = this.readMessage(type);
const schema = message && message.header();
const schema = message?.header();
if (throwIfNull && !schema) {
throw new Error(nullMessage(type));
}
Expand All @@ -81,7 +81,7 @@ export class MessageReader implements IterableIterator<Message> {
protected readMetadataLength(): IteratorResult<number> {
const buf = this.source.read(PADDING);
const bb = buf && new ByteBuffer(buf);
const len = bb && bb.readInt32(0) || 0;
const len = bb?.readInt32(0) || 0;
return { done: len === 0, value: len };
}
protected readMetadata(metadataLength: number): IteratorResult<Message> {
Expand Down Expand Up @@ -141,7 +141,7 @@ export class AsyncMessageReader implements AsyncIterableIterator<Message> {
public async readSchema(throwIfNull = false) {
const type = MessageHeader.Schema;
const message = await this.readMessage(type);
const schema = message && message.header();
const schema = message?.header();
if (throwIfNull && !schema) {
throw new Error(nullMessage(type));
}
Expand All @@ -150,7 +150,7 @@ export class AsyncMessageReader implements AsyncIterableIterator<Message> {
protected async readMetadataLength(): Promise<IteratorResult<number>> {
const buf = await this.source.read(PADDING);
const bb = buf && new ByteBuffer(buf);
const len = bb && bb.readInt32(0) || 0;
const len = bb?.readInt32(0) || 0;
return { done: len === 0, value: len };
}
protected async readMetadata(metadataLength: number): Promise<IteratorResult<Message>> {
Expand Down Expand Up @@ -220,7 +220,7 @@ export class JSONMessageReader extends MessageReader {
public readSchema() {
const type = MessageHeader.Schema;
const message = this.readMessage(type);
const schema = message && message.header();
const schema = message?.header();
if (!message || !schema) {
throw new Error(nullMessage(type));
}
Expand Down
8 changes: 4 additions & 4 deletions js/src/ipc/reader.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ class RecordBatchFileReaderImpl<T extends { [key: string]: DataType } = any> ext
const block = this._footer && this._footer.getRecordBatch(index);
if (block && this._handle.seek(block.offset)) {
const message = this._reader.readMessage(MessageHeader.RecordBatch);
if (message && message.isRecordBatch()) {
if (message?.isRecordBatch()) {
const header = message.header();
const buffer = this._reader.readMessageBody(message.bodyLength);
const recordBatch = this._loadRecordBatch(header, buffer);
Expand All @@ -560,7 +560,7 @@ class RecordBatchFileReaderImpl<T extends { [key: string]: DataType } = any> ext
const block = this._footer && this._footer.getDictionaryBatch(index);
if (block && this._handle.seek(block.offset)) {
const message = this._reader.readMessage(MessageHeader.DictionaryBatch);
if (message && message.isDictionaryBatch()) {
if (message?.isDictionaryBatch()) {
const header = message.header();
const buffer = this._reader.readMessageBody(message.bodyLength);
const vector = this._loadDictionaryBatch(header, buffer);
Expand Down Expand Up @@ -621,7 +621,7 @@ class AsyncRecordBatchFileReaderImpl<T extends { [key: string]: DataType } = any
const block = this._footer && this._footer.getRecordBatch(index);
if (block && (await this._handle.seek(block.offset))) {
const message = await this._reader.readMessage(MessageHeader.RecordBatch);
if (message && message.isRecordBatch()) {
if (message?.isRecordBatch()) {
const header = message.header();
const buffer = await this._reader.readMessageBody(message.bodyLength);
const recordBatch = this._loadRecordBatch(header, buffer);
Expand All @@ -634,7 +634,7 @@ class AsyncRecordBatchFileReaderImpl<T extends { [key: string]: DataType } = any
const block = this._footer && this._footer.getDictionaryBatch(index);
if (block && (await this._handle.seek(block.offset))) {
const message = await this._reader.readMessage(MessageHeader.DictionaryBatch);
if (message && message.isDictionaryBatch()) {
if (message?.isDictionaryBatch()) {
const header = message.header();
const buffer = await this._reader.readMessageBody(message.bodyLength);
const vector = this._loadDictionaryBatch(header, buffer);
Expand Down
2 changes: 1 addition & 1 deletion js/src/table.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ export class Table<T extends { [key: string]: DataType } = any>

const chunks = selectArgs<RecordBatch<T>>(RecordBatch, args);

if (!schema && !(schema = chunks[0] && chunks[0].schema)) {
if (!schema && !(schema = chunks[0]?.schema)) {
throw new TypeError('Table must be initialized with a Schema or at least one RecordBatch');
}

Expand Down
Loading

0 comments on commit 951f476

Please sign in to comment.