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

[WIP] build: generate commonjs, es module, and umd versions of unified #76

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,5 @@
.nyc_output/
coverage/
node_modules/
unified.js
unified.min.js
dist/
yarn.lock
5 changes: 3 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ deploy:
api_key:
secure: A16yUNG+m/p98L0mFLIy4IYWJsjWOKujHGEnJ2SwwIGvndLCtIig1BTp3C9Wkwf356UfHwcsPSa95bnpHXEsNZZwsVfzF4Hb2Csn0A9JKrpzoc+VdSgSDl+rUoq71x1TnLn4B2sC6k5/webcU26GBtVfeLka47OIUtuzcn8sxWHDr1JgRtSfVsasO4Iw21bSUnKKrIcgVLBoMrjRbJdCt3yzGaWbr/GuEQ/kXJfFY8zYh1iaJ12Nkqrof1lV766TJPL585uiuXQaHv0u+Hq6MNdKQqN1Tb+CgWB9VJrJaN/DxZpKfsl3FkO21c7UokvWIGP9WV/z5JVDcWhjfH7qRk+joMPYpHzUAlFnOWAz/PTWo0o65hKMrQKSYRxVsmWDSE+0EUV79mOaSUasEPHihXuwzqxrJtkCFW0rAs46Jvu0Z8hLtznH8FuiDU9l3BZsQA8/V5dFJ0Z6yLbvhs0Hwv/+IrLVpeDRixwaaar3EL6RE2zKAharSEgUw78lLkqSI6cDHVsIZVp22kAoeXB3m2JPAO0AAf1EbgZW1RA0kNb8gX3e5AoDsDgRb4ULlCCVXE6EXkH8lfa79Yr+7AUNk3v8orXJ5Qkofzby6js+t3qyTV8+i5T2vHj35lE+hq2KdH5LRVi2+0cmOP+8suJcUq3Two8TKPQXmyIsBOnSQBc=
file:
- 'unified.js'
- 'unified.min.js'
- 'dist/unified.js'
- 'dist/unified.mjs'
- 'dist/unified.umd.js'
on:
tags: true
33 changes: 16 additions & 17 deletions index.js
Original file line number Diff line number Diff line change
@@ -1,22 +1,10 @@
'use strict'

var extend = require('extend')
var bail = require('bail')
var vfile = require('vfile')
var trough = require('trough')
var plain = require('is-plain-obj')

// Expose a frozen processor.
module.exports = unified().freeze()

var slice = [].slice
var own = {}.hasOwnProperty

// Process pipeline.
var pipeline = trough()
.use(pipelineParse)
.use(pipelineRun)
.use(pipelineStringify)
import extend from 'extend'
import bail from 'bail'
import vfile from 'vfile'
import trough from 'trough'
import plain from 'is-plain-obj'

function pipelineParse(p, ctx) {
Copy link
Member

Choose a reason for hiding this comment

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

For some reason coverage doesn’t see this function as covered?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is likely also related to bcoe/c8#66

ctx.tree = p.parse(ctx.file)
Expand Down Expand Up @@ -131,6 +119,8 @@ function unified() {
// Data management.
// Getter / setter for processor-specific informtion.
function data(key, value) {
var own = {}.hasOwnProperty

if (typeof key === 'string') {
// Set `key`.
if (arguments.length === 2) {
Expand Down Expand Up @@ -230,6 +220,7 @@ function unified() {

function addPlugin(plugin, value) {
var entry = find(plugin)
var slice = [].slice

if (entry) {
if (plain(entry[1]) && plain(value)) {
Expand Down Expand Up @@ -361,6 +352,11 @@ function unified() {

function executor(resolve, reject) {
var file = vfile(doc)
// Process pipeline.
var pipeline = trough()
.use(pipelineParse)
.use(pipelineRun)
.use(pipelineStringify)
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn’t this make every run significantly slower?

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe, benchmarking would give better insights on the potential impact.


pipeline.run(processor, {file: file}, done)

Expand Down Expand Up @@ -461,3 +457,6 @@ function assertDone(name, asyncName, complete) {
)
}
}

// Expose a frozen processor.
export default unified().freeze()
Copy link
Member Author

Choose a reason for hiding this comment

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

This was moved to the end due to bcoe/c8#66

26 changes: 11 additions & 15 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,16 @@
"Vse Mozhet Byt <[email protected]>",
"Richard Littauer <[email protected]>"
],
"source": "index.js",
"main": "dist/unified.js",
"module": "dist/unified.mjs",
"unpkg": "dist/unified.umd.js",
"types": "types/index.d.ts",
Comment on lines +28 to 32
Copy link
Member Author

Choose a reason for hiding this comment

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

These are the different build types recommended by microbundle
https://github.com/developit/microbundle#set-up-your-packagejson

The recommendations also line up pretty well with Babel's recommendations
https://babeljs.io/blog/2018/06/26/on-consuming-and-publishing-es2015+-packages

"sideEffects": false,
Copy link
Member Author

Choose a reason for hiding this comment

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

this flag is specifically for bundlers, it signals that additional tree-shaking optimizations can be safely made.
https://webpack.js.org/guides/tree-shaking/

"files": [
"types/index.d.ts",
"index.js",
"lib"
"dist"
],
"dependencies": {
"bail": "^1.0.0",
Expand All @@ -40,32 +45,23 @@
},
"devDependencies": {
"browserify": "^16.0.0",
"c8": "^6.0.1",
"dtslint": "^1.0.2",
"nyc": "^14.0.0",
"microbundle": "^0.11.0",
"prettier": "^1.0.0",
"remark-cli": "^7.0.0",
"remark-preset-wooorm": "^6.0.0",
"tape": "^4.0.0",
"tinyify": "^2.5.1",
"typescript": "^3.0.0",
"xo": "^0.25.0"
},
"scripts": {
"format": "remark . -qfo && prettier --write \"**/{*.js,*.ts}\" && xo --fix",
"build-bundle": "browserify index.js -s unified -o unified.js",
"build-mangle": "browserify index.js -s unified -p tinyify -o unified.min.js",
"build": "npm run build-bundle && npm run build-mangle",
"format": "remark . -qfo && prettier --write \"index.js\" \"types/**\" && xo --fix",
"build": "microbundle",
"test-api": "node test",
"test-coverage": "nyc --reporter lcov tape test",
"test-coverage": "c8 --reporter=lcov --check-coverage --lines 100 --functions 100 --branches 100 tape test",
Copy link
Member Author

Choose a reason for hiding this comment

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

also migrate coverage to c8, which leverages V8's native coverage capabilities, since the builds now are done on Node 10+
https://github.com/bcoe/c8

May open a separate PR just for this.

Copy link
Member

Choose a reason for hiding this comment

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

C8 seems to be pretty young, unstable (see pipelineParse), and include bugs. What’s the benefit of switching?

Copy link
Member

Choose a reason for hiding this comment

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

(oh and see: #76 (comment))

Copy link
Member Author

Choose a reason for hiding this comment

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

C8 seems to be pretty young, unstable

Young-ish, 1.0 was released in Oct 25, 2017, over two years ago.
It is as stable as the coverage feature in v8.

and includes bugs

🤷‍♂️ so do Nyc and Istanbul

What’s the benefit of switching?

It is a native feature of Chrome/Node and should more accurately reflect runtime coverage, than the build-instrumentation based approach of Instanbul (some work may still be needed), as well has having the performance boost from running directly in the platform.

"test-types": "dtslint types",
"test": "npm run format && npm run build && npm run test-coverage && npm run test-types"
},
"nyc": {
"check-coverage": true,
"lines": 100,
"functions": 100,
"branches": 100
},
"prettier": {
"tabWidth": 2,
"useTabs": false,
Expand Down