[v2.0.0] Bundle chokidar #3331

lukastaegert · 2020-01-12T19:07:40Z

This PR contains:

Are tests included?

yes (bugfixes and features will not be merged without tests)
no

Breaking Changes?

yes (breaking changes will not be merged unless absolutely necessary)
no

List any relevant issue numbers:
Resolves #2988

Description

This is the first PR in preparation of the 2.0.0 release: https://github.com/rollup/rollup/projects/2
This will likely receive some additional polishing and testing before it will be merged to the new release-2.0.0 branch. There may be pre-releases from the branch if there is interest once the first changes are merged.

This will make Chokidar the only watcher used by Rollup and bundle it. Also, the code was slightly restructured so that all watch mode related code is in separate chunks that are only loaded on demand via dynamic import (or conditional require in the CJS build). This means that

When running Rollup, the two biggest bundled dependencies chokidar and micromatch are only loaded and parsed when watch mode is used
JavaScript API users of Rollup that bundle Rollup into their own project do not run risk of bundling these dependencies as the dynamic imports can easily be treeshaken

Bundling chokidar itself was mainly painless. However, its fsevents dependency cannot be bundled as it contains native code. Therefore, it became an optional dependency of Rollup. There is a check in place to make sure its version range matches that of chokidar.

In order to handle the optional require of fsevents in the Rollup code, I created a loader file, patched the chokidar code to load the loader file instead of fsevents and in that loader file, again used a dynamic import to load fsevents. As the watcher itself is already loaded via a dynamic import, I chained these imports so that chokidar will now behave the same no matter if fsevents is present and valid or missing.

codecov · 2020-01-13T06:35:40Z

Codecov Report

❗ No coverage uploaded for pull request base (release-2.0.0@72f1ade). Click here to learn what that means.
The diff coverage is 94.44%.

@@               Coverage Diff                @@
##             release-2.0.0    #3331   +/-   ##
================================================
  Coverage                 ?   93.49%           
================================================
  Files                    ?      173           
  Lines                    ?     6013           
  Branches                 ?     1787           
================================================
  Hits                     ?     5622           
  Misses                   ?      211           
  Partials                 ?      180

Impacted Files	Coverage Δ
src/Graph.ts	`92.94% <ø> (ø)`
src/utils/PluginContext.ts	`100% <ø> (ø)`
src/utils/PluginDriver.ts	`100% <ø> (ø)`
src/rollup/rollup.ts	`96.95% <100%> (ø)`
src/watch/watch.ts	`97.97% <100%> (ø)`
src/watch/watch-proxy.ts	`100% <100%> (ø)`
src/watch/fsevents-importer.ts	`40% <40%> (ø)`
src/watch/fileWatcher.ts	`96.42% <96.42%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 72f1ade...6db5c94. Read the comment docs.

lukastaegert · 2020-01-17T15:34:48Z

Ok, this was quite a piece of work. Apparently, there is an issue in chokidar when a file is "updated" by unlinking and then writing it again in that on certain OSs, specifically our Linux images, the file is no longer watched after that. I fixed it by "unwatch"ing and immediately "add"ing again any changed file.

I also completely rewrote and simplified the watcher code to fully take advantage of chokidar. Hopefully this will reduce memory consumption and improve the experience for everyone!

This is now ready from my side.

lukastaegert · 2020-01-17T16:27:58Z

I created paulmillr/chokidar#972 to track the issue in chokidar I discovered. This should not block release, though, as a workaround is in place.

paulmillr · 2020-01-19T00:11:03Z

@lukastaegert I suggest to only do unwatch and add if os.platform() is linux — only. Otherwise you'll see unnecessary performance degradation.

…kidar depends on utils.promisify

…rmance

* Create a separate chunk for watch mode via a dynamic import * Bundle chokidar * Handle conditional fsevents import * Only run tests on Node 10 and 12 as we want to do this anyway and chokidar depends on utils.promisify * Improve chunking * Refactor and simplify watcher * Make watcher a proper parameter * Only unwatch and watch on Linux systems * No longer run JS files through TypeScript for improved bundling performance

* [v2.0.0] Update minimum required Node version to 10 (#3346) * Update CI to only run on Node 10 and 12, add engines to package.json * Update TypeScript target * Remove Bublé as a transpilation option from tests (it is still needed in sourcemap tests) * Remove minNodeVersion form tests * Reformat package.json * [v2.0.0] Bundle chokidar (#3331) * Create a separate chunk for watch mode via a dynamic import * Bundle chokidar * Handle conditional fsevents import * Only run tests on Node 10 and 12 as we want to do this anyway and chokidar depends on utils.promisify * Improve chunking * Refactor and simplify watcher * Make watcher a proper parameter * Only unwatch and watch on Linux systems * No longer run JS files through TypeScript for improved bundling performance * [v2.0.0] Remove active deprecations (#3352) * Remove active deprecations * Make inactive deprecations active * Improve coverage * Improve coverage * Refine transformer * Improve coverage * Refactoring option normalization * Improve coverage * Remove experimentalOptimizeChunks * Remove more unused code