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.

Sign up for GitHub

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

Jump to bottom

Watch target files with Chokidar #30

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Watch target files with Chokidar #30

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
21ef49e
Added initial file watching with Chokidar
blake-mealey Nov 3, 2019
53e7c6f
Updated yarn.lock for chokidar
blake-mealey Nov 3, 2019
709edab
Removed debug logging
blake-mealey Nov 4, 2019
89a6b88
cleanup watchFiles
blake-mealey Nov 4, 2019
cdfda05
Added unit tests
blake-mealey Nov 5, 2019
1755733
Renamed restPluginOptions -> copyOptions in copyFiles
blake-mealey Nov 5, 2019
d416b01
Renamed restPluginOptions -> copyOptions in watchFiles
blake-mealey Nov 5, 2019
4d6431c
Added multiple targets watch test
blake-mealey Nov 5, 2019
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
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
},
"dependencies": {
"@types/fs-extra": "^8.0.0",
"chokidar": "^3.3.0",
"colorette": "^1.1.0",
"fs-extra": "^8.1.0",
"globby": "10.0.1",
Expand Down
98 changes: 68 additions & 30 deletions src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import fs from 'fs-extra'
import isObject from 'is-plain-object'
import globby from 'globby'
import { bold, green, yellow } from 'colorette'
import chokidar from 'chokidar'

function stringify(value) {
return util.inspect(value, { breakLength: Infinity })
Expand All @@ -27,6 +28,63 @@ function generateCopyTarget(src, dest, rename) {
}
}

function generateCopyTargets(src, dest, rename) {
blake-mealey marked this conversation as resolved.
Show resolved Hide resolved
return Array.isArray(dest)
? dest.map(destination => generateCopyTarget(src, destination, rename))
: [generateCopyTarget(src, dest, rename)]
}

async function copyFiles(copyTargets, verbose, copyOptions) {
if (Array.isArray(copyTargets) && copyTargets.length) {
if (verbose) {
console.log(green('copied:'))
}

for (const { src, dest } of copyTargets) {
await fs.copy(src, dest, copyOptions)

if (verbose) {
console.log(green(` ${bold(src)} → ${bold(dest)}`))
}
}
} else if (verbose) {
console.log(yellow('no items to copy'))
}
}

function watchFiles(targets, verbose, copyOptions) {
return targets.map(({ src, dest, rename }) => {
async function onChange(matchedPath) {
const copyTargets = generateCopyTargets(matchedPath, dest, rename)
await copyFiles(copyTargets, verbose, copyOptions)
}

return chokidar.watch(src, { ignoreInitial: true })
vladshcherbin marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Owner

Choose a reason for hiding this comment

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

Unfortunately, we can't pass src to chokidar. Chokidar v3 uses anymatch, which uses picomatch inside:

chokidar -> anymatch -> picomatch

This package uses globby inside for glob support. Globby uses fast-glob inside, which uses micromatch:

globby -> fast-glob -> micromatch -> picomatch

So, chokidar v3 misses micromatch package which adds support for some glob features on top of picomatch. Globs will have different features for copy and watch. Chokidar used micromatch in v2, but switched in v3.

Copy link
Author

Choose a reason for hiding this comment

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

Well that's unfortunate... I'll think about it a bit and do some research into options.

Copy link
Owner

Choose a reason for hiding this comment

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

I've opened an issue in chokidar paulmillr/chokidar#956, maybe only braces extension is missing and we can live with that.

Copy link
Owner

Choose a reason for hiding this comment

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

Seems to be compatible. I'll add few tests this week and hopefully release a new version after merging this.

.on('change', onChange)
.on('add', onChange)
})
}

function verifyTargets(targets) {
if (Array.isArray(targets) && targets.length) {
for (const target of targets) {
if (!isObject(target)) {
throw new Error(`${stringify(target)} target must be an object`)
}

const { src, dest, rename } = target

if (!src || !dest) {
throw new Error(`${stringify(target)} target must have "src" and "dest" properties`)
}

if (rename && typeof rename !== 'string' && typeof rename !== 'function') {
throw new Error(`${stringify(target)} target's "rename" property must be a string or a function`)
}
}
}
}

export default function copy(options = {}) {
const {
copyOnce = false,
Expand All @@ -37,6 +95,9 @@ export default function copy(options = {}) {
} = options

let copied = false
let watchers = []

verifyTargets(targets)

return {
name: 'copy',
Expand All @@ -49,20 +110,8 @@ export default function copy(options = {}) {

if (Array.isArray(targets) && targets.length) {
for (const target of targets) {
if (!isObject(target)) {
throw new Error(`${stringify(target)} target must be an object`)
}

const { src, dest, rename, ...restTargetOptions } = target

if (!src || !dest) {
throw new Error(`${stringify(target)} target must have "src" and "dest" properties`)
}

if (rename && typeof rename !== 'string' && typeof rename !== 'function') {
throw new Error(`${stringify(target)} target's "rename" property must be a string or a function`)
}

const matchedPaths = await globby(src, {
expandDirectories: false,
onlyFiles: false,
Expand All @@ -72,33 +121,22 @@ export default function copy(options = {}) {

if (matchedPaths.length) {
matchedPaths.forEach((matchedPath) => {
const generatedCopyTargets = Array.isArray(dest)
? dest.map(destination => generateCopyTarget(matchedPath, destination, rename))
: [generateCopyTarget(matchedPath, dest, rename)]

copyTargets.push(...generatedCopyTargets)
copyTargets.push(...generateCopyTargets(matchedPath, dest, rename))
})
}
}
}

if (copyTargets.length) {
if (verbose) {
console.log(green('copied:'))
}

for (const { src, dest } of copyTargets) {
await fs.copy(src, dest, restPluginOptions)
await copyFiles(copyTargets, verbose, restPluginOptions)

if (verbose) {
console.log(green(` ${bold(src)} → ${bold(dest)}`))
}
}
} else if (verbose) {
console.log(yellow('no items to copy'))
if (!copied && !copyOnce && process.env.ROLLUP_WATCH === 'true') {
watchers = watchFiles(targets, verbose, restPluginOptions)
}

copied = true
},
_closeWatchers: async () => { // For unit tests
await Promise.all(watchers.map(watcher => watcher.close()))
}
}
}
144 changes: 139 additions & 5 deletions tests/index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { rollup, watch } from 'rollup'
import fs from 'fs-extra'
import replace from 'replace-in-file'
import { bold, yellow, green } from 'colorette'
import { join } from 'path'
import copy from '../src'

process.chdir(`${__dirname}/fixtures`)
Expand All @@ -16,12 +17,14 @@ afterEach(async () => {
})

async function build(options) {
const copyPlugin = copy(options)
await rollup({
input: 'src/index.js',
plugins: [
copy(options)
copyPlugin
]
})
return copyPlugin
}

describe('Copy', () => {
Expand Down Expand Up @@ -230,6 +233,137 @@ describe('Copy', () => {
})
})

describe('Watching', () => {
test('Does not watch target files when watch mode disabled', async () => {
await build({
targets: [
{ src: 'src/assets/asset-1.js', dest: 'dist' }
]
})

expect(await fs.pathExists('dist/asset-1.js')).toBe(true)
await fs.remove('dist')
expect(await fs.pathExists('dist/asset-1.js')).toBe(false)

await replace({
files: 'src/assets/asset-1.js',
from: 'asset1',
to: 'assetX'
})

await sleep(1000)

expect(await fs.pathExists('dist/asset-1.js')).toBe(false)

await replace({
files: 'src/assets/asset-1.js',
from: 'assetX',
to: 'asset1'
})
})

test('Does not watch target files when watch mode and copyOnce enabled', async () => {
process.env.ROLLUP_WATCH = 'true'
await build({
targets: [
{ src: 'src/assets/asset-1.js', dest: 'dist' }
],
copyOnce: true
})
delete process.env.ROLLUP_WATCH

expect(await fs.pathExists('dist/asset-1.js')).toBe(true)
await fs.remove('dist')
expect(await fs.pathExists('dist/asset-1.js')).toBe(false)

await replace({
files: 'src/assets/asset-1.js',
from: 'asset1',
to: 'assetX'
})

await sleep(1000)

expect(await fs.pathExists('dist/asset-1.js')).toBe(false)

await replace({
files: 'src/assets/asset-1.js',
from: 'assetX',
to: 'asset1'
})
})

test('Watches target files when watch mode enabled', async () => {
process.env.ROLLUP_WATCH = 'true'
const copyPlugin = await build({
targets: [
{ src: 'src/assets/asset-1.js', dest: 'dist' }
]
})
delete process.env.ROLLUP_WATCH

expect(await fs.pathExists('dist/asset-1.js')).toBe(true)
await fs.remove('dist')
expect(await fs.pathExists('dist/asset-1.js')).toBe(false)

await replace({
files: 'src/assets/asset-1.js',
from: 'asset1',
to: 'assetX'
})

await sleep(1000)

expect(await fs.pathExists('dist/asset-1.js')).toBe(true)

// eslint-disable-next-line no-underscore-dangle
await copyPlugin._closeWatchers()

await replace({
files: 'src/assets/asset-1.js',
from: 'assetX',
to: 'asset1'
})
})

test('Watches and copies multiple targets from same file', async () => {
process.env.ROLLUP_WATCH = 'true'
const copyPlugin = await build({
targets: [
{ src: 'src/assets/asset-1.js', dest: 'dist' },
{ src: 'src/assets/asset-1.js', dest: 'dist/2' }
]
})
delete process.env.ROLLUP_WATCH

expect(await fs.pathExists('dist/asset-1.js')).toBe(true)
expect(await fs.pathExists('dist/2/asset-1.js')).toBe(true)
await fs.remove('dist')
expect(await fs.pathExists('dist/asset-1.js')).toBe(false)
expect(await fs.pathExists('dist/2/asset-1.js')).toBe(false)

await replace({
files: 'src/assets/asset-1.js',
from: 'asset1',
to: 'assetX'
})

await sleep(1000)

expect(await fs.pathExists('dist/asset-1.js')).toBe(true)
expect(await fs.pathExists('dist/2/asset-1.js')).toBe(true)

// eslint-disable-next-line no-underscore-dangle
await copyPlugin._closeWatchers()

await replace({
files: 'src/assets/asset-1.js',
from: 'assetX',
to: 'asset1'
})
})
})

describe('Options', () => {
/* eslint-disable no-console */
test('Verbose', async () => {
Expand All @@ -251,16 +385,16 @@ describe('Options', () => {
expect(console.log).toHaveBeenCalledTimes(5)
expect(console.log).toHaveBeenCalledWith(green('copied:'))
expect(console.log).toHaveBeenCalledWith(
green(` ${bold('src/assets/asset-1.js')} → ${bold('dist/asset-1.js')}`)
green(` ${bold('src/assets/asset-1.js')} → ${bold(join('dist', 'asset-1.js'))}`)
Copy link
Author

Choose a reason for hiding this comment

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

This test was broken on Windows because it was getting 'dist\asset-1.js' instead of 'dist/asset-1.js'

join fixes this by using the separator from the OS

Copy link
Owner

Choose a reason for hiding this comment

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

Interesting, so it outputs backslashes on windows 🤔

I'll try to resolve this in src code so paths won't be different in mac/windows since this doesn't match with target.src ones.

)
expect(console.log).toHaveBeenCalledWith(
green(` ${bold('src/assets/css/css-1.css')} → ${bold('dist/css-1.css')}`)
green(` ${bold('src/assets/css/css-1.css')} → ${bold(join('dist', 'css-1.css'))}`)
)
expect(console.log).toHaveBeenCalledWith(
green(` ${bold('src/assets/css/css-2.css')} → ${bold('dist/css-2.css')}`)
green(` ${bold('src/assets/css/css-2.css')} → ${bold(join('dist', 'css-2.css'))}`)
)
expect(console.log).toHaveBeenCalledWith(
green(` ${bold('src/assets/scss')} → ${bold('dist/scss')}`)
green(` ${bold('src/assets/scss')} → ${bold(join('dist', 'scss'))}`)
)
})

Expand Down
Loading