keichi / binary-parser

A blazing-fast declarative parser builder for binary data
MIT License
864 stars 134 forks source link

chore: set shared tsc target (ES6) #179

Closed manzt closed 2 years ago

manzt commented 2 years ago

The commonjs & esm builds have different tsc targets ('es5' & 'esnext'). The javascript class was introduced in es6 (I believe), so the Parsers exported from the cjs & esm builds are different due to transpilation and have different characteristics. Example:

import { Parser } from 'binary-parser'; // esm
class MyParser extends Parser { ... } // works because Parser is a javascript class and can be extended
const { Parser } = require('binary-parser');
class MyParser extends Parser { ... } // Parser isn't a class for cjs output so this doesn't work correctly..

~This PR sets the target output to es2018, which is a modern target and 100% compatible with node >= 10. This way the only difference between the CJS & ESM builds is the module system.~

The target has been changed to ES6.

keichi commented 2 years ago

Would there be a problem if we target older ES versions (i.e. ES5 or ES6)? It seems most popular TypeScript projects are targeting either ES5 or ES6.

https://github.com/angular/angular/blob/master/tools/tsconfig.json https://github.com/mui-org/material-ui/blob/master/tsconfig.json https://github.com/evanw/esbuild/blob/master/lib/tsconfig.json

manzt commented 2 years ago

I don't have an issue with targeting a slightly older ES version. However, I would push for ES6 (ES2015) because it is well supported in Node and browsers, and this specifically includes class keyword. If needed, one can transpile the library to an earlier target (and would need since ESM was introduced in ES6), and additionally polyfill TextDecoder & BigInt for that environment.

Full disclosure, I am considering extending Parser in a personal project via class Parser extends BaseParser {}.

manzt commented 2 years ago

Additionally, as of TS 4.5 apparently the default for tsc --init is ES7 (ES2016): https://twitter.com/orta/status/1441468054689419265

keichi commented 2 years ago

Ok, let's target ES6 then :+1: