apla
commented
5 years ago Babel is too much for this task. Modern way to me is esm +rollup + buble.
Open nezed opened 5 years ago
apla
commented
5 years ago Babel is too much for this task. Modern way to me is esm +rollup + buble.
nezed
commented
5 years ago @apla You proposed good stack of tools, but lets highlight possible problems that we've trying to solve:
babel-preset-env has option useBuiltIns: 'usage' that produces babel to add only required polyfils. Also that polyfills based on core-js has additional runtime feature-detection which minimizes overrides of native methods.
(e.g. Promise polyfill not even included in builds for node@10, for node@6+ it included with runtime checks, and for node@0 it always enabled)
As i understand, Buble does not provides any polyfills support, just transforms.
A lot of ES6+ are supported by nodejs for now, e.g. Object/arguments spread & rest and doesn't require transformations.
babel-preset-env allows you to set only wanted environments, and only required transformations will be applied
(e.g. const operator doesn't transforms into var when building for node@8)
Buble just transforms all known features by preconfigured list of transforms
A lot of code in this repo can be simplified with async/await. Babel has great support for it for many years.
Buble has interesting philosophy and good set of transform, but i dont thins that its enough.
This is the rich nodejs library, which does not requires bundling. All imports working pretty good, even duplicated imports are handled by nodejs itself
This is the project with simple file structure, and es-modules doesn't gave any benefits (except beautiful syntax). End-user library code does not relays on a big set of dependencies, so there is nothing to drop with tree-shaking
Babel are stable project, with large community. It easy to setup and maintain (look at the config in this PR - its complete setup with Babel)
codecov-io
commented
5 years ago Merging #38 into master will increase coverage by
6.4%. The diff coverage is100%.
@@ Coverage Diff @@
## master #38 +/- ##
=========================================
+ Coverage 87.81% 94.21% +6.4%
=========================================
Files 5 4 -1
Lines 320 294 -26
=========================================
- Hits 281 277 -4
+ Misses 39 17 -22| Impacted Files | Coverage Δ | |
|---|---|---|
| src/clickhouse.js | 96.22% <100%> (ø) |
:arrow_up: |
| src/streams.js | 98.75% <0%> (-0.02%) |
:arrow_down: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update ecb2f06...ae15680. Read the comment docs.
nezed
commented
5 years ago I've checked current setup with dummy npm publish and it looks finished (See "Tarball Contents")
▶ npm publish
> @apla/clickhouse@1.6.0 prepublishOnly .
> npm run build
> @apla/clickhouse@1.6.0 build /Users/nezed/projects/node-clickhouse
> rm -r lib*/; babel src -d lib --source-maps && BABEL_ENV=legacy babel src -d lib-legacy --source-maps
Successfully compiled 4 files with Babel.
Successfully compiled 4 files with Babel.
npm notice
npm notice 📦 @apla/clickhouse@1.6.0
npm notice === Tarball Contents ===
npm notice 1.6kB package.json
npm notice 442B index.js
npm notice 1.1kB LICENSE
npm notice 8.2kB README.md
npm notice 10.5kB lib-legacy/clickhouse.js
npm notice 21.9kB lib-legacy/clickhouse.js.map
npm notice 1.5kB lib-legacy/parse-error.js
npm notice 2.6kB lib-legacy/parse-error.js.map
npm notice 5.6kB lib-legacy/process-db-value.js
npm notice 8.0kB lib-legacy/process-db-value.js.map
npm notice 4.9kB lib-legacy/streams.js
npm notice 10.4kB lib-legacy/streams.js.map
npm notice 9.9kB lib/clickhouse.js
npm notice 21.9kB lib/clickhouse.js.map
npm notice 1.1kB lib/parse-error.js
npm notice 2.6kB lib/parse-error.js.map
npm notice 4.6kB lib/process-db-value.js
npm notice 8.0kB lib/process-db-value.js.map
npm notice 4.8kB lib/streams.js
npm notice 10.4kB lib/streams.js.map
npm notice === Tarball Details ===
npm notice name: @apla/clickhouse
npm notice version: 1.6.0
npm notice package size: 38.7 kB
npm notice unpacked size: 139.9 kB
npm notice shasum: f5c2cc81a2c3632ccad21cb3a17b8404cd6fb90f
npm notice integrity: sha512-j6ffxHd4k4Nkm[...]smrZBb+n5Coug==
npm notice total files: 20
npm notice
npm ERR! code E404
npm ERR! 404 Not Found - PUT https://registry.npmjs.org/@apla%2fclickhouse - Not found
npm ERR! 404
npm ERR! 404 '@apla/clickhouse@1.6.0' is not in the npm registry.
npm ERR! 404 You should bug the author to publish it (or use the name yourself!)
A lot of ES6+ syntax features like
const,async/await, spread operator and etc are supported in node@6.5+.Node.js release calendar says that maintenance of node@6 LTS was ended at may of 2019. This means that all versions earlier node@8 become obsolete.
I'm proposing to do different code builds for legacy and actual nodejs version