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

fix: Rollup rebuilds in watch mode #449

Merged
merged 28 commits into from
Jul 12, 2018
Merged

fix: Rollup rebuilds in watch mode #449

merged 28 commits into from
Jul 12, 2018

Conversation

tivac
Copy link
Owner

@tivac tivac commented Jul 12, 2018

This is all fallout from the switch to using dependencies.

The transform hook is called in three different ways.

  1. The file is being transformed for the first time
  2. The file changed
  3. One of the dependencies of the file changed

Previously the same codepath handled cases 1 & 2, but that was very wrong. Now they're separated out!

@TravisBuddy
Copy link

Travis tests have failed

Hey @tivac,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

Node.js: 8

npm test -- --verbose --ci
> modular-css-root@0.0.0 pretest /home/travis/build/tivac/modular-css
> npm run parser


> modular-css-root@0.0.0 parser /home/travis/build/tivac/modular-css
> pegjs packages/core/parsers/parser.pegjs


> modular-css-root@0.0.0 test /home/travis/build/tivac/modular-css
> jest "--verbose" "--ci"

PASS packages/core/test/options.test.js
  /processor.js
    options
      cwd
        ✓ should use an absolute path (424ms)
        ✓ should accept a relative path but make it absolute (3ms)
      namer
        ✓ should use a custom naming function (5ms)
        ✓ should require a namer if a string is passed (32ms)
        ✓ should use the default naming function if a non-function is passed (1ms)
      map
        ✓ should generate source maps (25ms)
        ✓ should generate external source maps (14ms)
      exportGlobals
        ✓ should not export :global values when exportGlobals is false (3ms)
      rewrite
        ✓ should rewrite url() references by default (5ms)
        ✓ should not rewrite url() references when falsey (2ms)
        ✓ should pass through to postcss-url as config (21ms)
      lifecycle options
        before
          ✓ should run sync postcss plugins before processing (3ms)
          ✓ should run async postcss plugins before processing (77ms)
        processing
          ✓ should run sync postcss plugins processing processing (2ms)
          ✓ should run async postcss plugins processing processing (2ms)
          ✓ should include exports from 'modular-css-export' modules (2ms)
        after
          ✓ should use postcss-url by default (2ms)
          ✓ should run sync postcss plugins (3ms)
          ✓ should run async postcss plugins (3ms)
        done
          ✓ should run sync postcss plugins done processing (2ms)
          ✓ should run async postcss plugins done processing (2ms)
          ✓ should work with cssnano (no preset) (1038ms)
          ✓ should work with cssnano (default preset) (13ms)

FAIL packages/rollup/test/rollup.test.js
  /rollup.js
    ✓ should be a function (2ms)
    ✓ should generate exports (54ms)
    ✓ should be able to tree-shake results (9ms)
    ✓ should generate CSS (109ms)
    ✕ should handle assetFileNames being undefined (34ms)
    ✓ should correctly pass to/from params for relative paths (9ms)
    ✓ should avoid generating empty CSS (3ms)
    ✓ should generate JSON (8ms)
    ✓ should provide named exports (8ms)
    ✓ should generate external source maps (8ms)
    ✓ should warn & not export individual keys when they are not valid identifiers (9ms)
    ✓ should allow disabling of named exports (6ms)
    ✓ shouldn't disable sourcemap generation (7ms)
    ✓ should not output sourcemaps when they are disabled (5ms)
    ✓ should respect the CSS dependency tree (8ms)
    ✓ should accept an existing processor instance (6ms)
    ✓ shouldn't over-remove files from an existing processor instance (13ms)
    errors
      ✓ should throw errors in in before plugins (2ms)
      ✓ should throw errors in after plugins (3ms)
      ○ skipped 1 test
    code splitting
      ✓ should support splitting up CSS files (9ms)
      ✓ should support manual chunks (20ms)
      ✓ should support dynamic imports (24ms)

  ● /rollup.js › should handle assetFileNames being undefined

    ReferenceError: output is not defined

       97 |         await bundle.write({
       98 |             format,
    >  99 |             file : `${output}/assetFileNames/simple.js`,
          |                       ^
      100 |         });
      101 | 
      102 |         const [ css ] = shell.ls(`${output}/assetFileNames/assets`);

      at Object.it (packages/rollup/test/rollup.test.js:99:23)

PASS packages/core/test/api.test.js
  /processor.js
    API
      ✓ should be a function (13ms)
      ✓ should auto-instantiate if called without new (1ms)
      .string()
        ✓ should process a string (4ms)
      .file()
        ✓ should process a relative file (3ms)
        ✓ should process an absolute file (2ms)
      .remove()
        ✓ should remove a relative file (2ms)
        ✓ should remove an absolute file (1ms)
        ✓ should remove multiple files (2ms)
        ✓ should return an array of removed files (1ms)
      .dependencies()
        ✓ should return the dependencies of the specified file (20ms)
        ✓ should return the overall order of dependencies if no file is specified (8ms)
      .dependents()
        ✓ should return the dependents of the specified file (8ms)
        ✓ should throw if no file is passed (12ms)
      .output()
        ✓ should return a postcss result (17ms)
        ✓ should generate css representing the output from all added files (14ms)
        ✓ should avoid duplicating files in the output (11ms)
        ✓ should generate a JSON structure of all the compositions (8ms)
        ✓ should order output by dependencies, then alphabetically (14ms)
        ✓ should support returning output for specified relative files (10ms)
        ✓ should support returning output for specified absolute files (9ms)
        ✓ should reject if called before input has been processed (9ms)
        ✓ should allow for seperate source map output (27ms)
      ._resolve()
        ✓ should run resolvers until a match is found (3ms)
        ✓ should fall back to a default resolver (5ms)

PASS packages/rollup/test/rollup-watch.test.js
  /rollup.js
    watch mode
      ✓ should generate updated output (439ms)
      ✓ should generate updated output for composes changes (456ms)
      ✓ should update when a dependency changes (503ms)
      ✓ should update when adding new css files (408ms)
      ✓ should update when a shared dependency changes (413ms)

(node:5325) DeprecationWarning: Tapable.plugin is deprecated. Use new API on `.hooks` instead
PASS packages/webpack/test/webpack.test.js
  /webpack.js
    ✓ should be a function (3ms)
    ✓ should output css to disk (268ms)
    ✓ should output json to disk (38ms)
    ✓ should output inline source maps (24ms)
    ✓ should output external source maps to disk (28ms)
    ✓ should report errors (25ms)
    ✓ should report warnings on invalid property names (30ms)
    ✓ should handle dependencies (54ms)
    ✓ should support ES2015 default exports (22ms)
    ✓ should support ES2015 named exports (20ms)
    ✓ should support disabling namedExports when the option is set (19ms)
    ✓ should generate correct builds in watch mode when files change (364ms)
    ✓ should generate correct builds when files change (53ms)

PASS packages/browserify/test/factor-bundle.test.js
  /browserify.js
    factor-bundle
      ✓ should be supported (372ms)
      ✓ should support files w/o commonalities (49ms)
      ✓ should properly handle files w/o dependencies (55ms)
      ✓ should support relative paths within factor-bundle files (73ms)
      ✓ should avoid outputting empty css files by default (54ms)
      ✓ should output empty css files when asked (58ms)

PASS packages/browserify/test/browserify.test.js
  /browserify.js
    basic functionality
      ✓ should not error if no options are supplied (17ms)
      ✓ should error if an invalid extension is applied (3ms)
      ✓ should error on invalid CSS (15ms)
      ✓ should replace require() calls with the exported identifiers (37ms)
      ✓ should correctly rewrite urls based on the destination file (37ms)
      ✓ should use the specified namer function (36ms)
      ✓ should include all CSS dependencies in output css (49ms)
      ✓ should write out the complete exported identifiers when `json` is specified (38ms)
      ✓ should not include duplicate files in the output multiple times (46ms)
      ✓ should output an inline source map when the debug option is specified (60ms)
      ✓ should output an external source map when the debug option is specified (45ms)

PASS packages/core/test/composition.test.js
  /processor.js
    composition
      ✓ should fail on invalid composes syntax (14ms)
      ✓ should fail if a composition references a non-existant class (3ms)
      ✓ should fail if a composition references a non-existant file (1ms)
      ✓ should fail if composes isn't the first property (2ms)
      ✓ should fail on rules that use multiple selectors (2ms)
      ✓ should compose a single class (3ms)
      ✓ should allow comments before composes (3ms)
      ✓ should compose from globals (2ms)
      ✓ should compose multiple classes (2ms)
      ✓ should compose from other files (5ms)

PASS packages/svelte/test/svelte.test.js
  /svelte.js
    ✓ should extract CSS from a <style> tag (52ms)
    ✓ should extract CSS from a <link> tag ("no script") (13ms)
    ✓ should extract CSS from a <link> tag ("existing script") (9ms)
    ✓ should extract CSS from a <link> tag ("single quotes") (9ms)
    ✓ should extract CSS from a <link> tag ("unquoted") (12ms)
    ✓ should ignore files without <style> blocks (5ms)
    ✓ should handle invalid references in "<script>" (inline: true, strict: true) (2ms)
    ✓ should handle invalid references in "template" (inline: true, strict: true) (2ms)
    ✓ should handle invalid references in "<script>" (inline: true, strict: false) (5ms)
    ✓ should handle invalid references in "template" (inline: true, strict: false) (3ms)
    ✓ should handle invalid references in "<script>" (inline: false, strict: false) (2ms)
    ✓ should handle invalid references in "template" (inline: false, strict: false) (2ms)
    ✓ should handle invalid references in "<script>" (inline: false, strict: true) (1ms)
    ✓ should handle invalid references in "template" (inline: false, strict: true) (2ms)
    ✓ should throw on both <style> and <link> in one file (1ms)

PASS packages/browserify/test/watchify.test.js
  /browserify.js
    watchify
      ✓ shouldn't cache file contents between watchify runs (511ms)
      ✓ shouldn't explode on invalid CSS (160ms)

PASS packages/core/test/values.test.js
  /processor.js
    values
      ✓ should fail on invalid value syntax (13ms)
      ✓ should fail if a value imports a non-existant reference (2ms)
      ✓ should support simple values (4ms)
      ✓ should support local values in value composition (9ms)
      ✓ should support importing variables from a file (6ms)
      ✓ should support exporting imported variables (7ms)
      ✓ should support value composition (5ms)
      ✓ should support value namespaces (5ms)
      ✓ should support value replacement in :external(...) (28ms)

PASS packages/postcss/test/postcss.test.js
  /postcss.js
    ✓ should be a function (2ms)
    ✓ should process CSS and output the result (28ms)
    ✓ should process CSS with dependencies and output the result (12ms)
    ✓ should process CSS and output exports as a message (2ms)
    ✓ should accept normal processor options (5ms)
    ✓ should accept a `json` property and write exports to that file (9ms)
    ✓ should be usable like a normal postcss plugin (3ms)
    ✓ should output json when used within postcss (2ms)
    ✓ should accept json args in either position with postcss (1ms)

PASS packages/aliases/test/aliases.test.js
  modular-css-aliases
    ✓ should return a falsey value if a file isn't found (2ms)
    ✓ should return the absolute path if a file is found (10ms)
    ✓ should check multiple aliases for files & return the first match (1ms)
    ✓ should be usable as a modular-css resolver (20ms)
    ✓ should fall through to the default resolver (7ms)

PASS packages/core/test/scoping.test.js
  /processor.js
    scoping
      ✓ should scope classes, ids, and keyframes (16ms)
      ✓ should handle pseudo classes correctly (4ms)
      ✓ should not allow :global classes to overlap with local ones (local before global) (1ms)
      ✓ should not allow :global classes to overlap with local ones (global before local) (1ms)
      ✓ should not allow empty :global() selectors (1ms)

PASS packages/core/test/keyframes.test.js
  /processor.js
    scoping
      ✓ should leave unknown animation names alone (19ms)
      ✓ should update scoped animations from the scoping plugin's message (3ms)
      ✓ should update the animation-name property (2ms)
      ✓ should update multiple animations properly (2ms)
      ✓ should update scoped prefixed animations from the scoping plugin's message (2ms)

PASS packages/browserify/test/issue-58.test.js
  /browserify.js
    /issues
      /58
        ✓ should update when CSS dependencies change (208ms)

PASS packages/browserify/test/issue-313.test.js
  /browserify.js
    /issues
      /313
        ✓ should include all dependencies after watchify update (207ms)

PASS packages/cli/test/cli.test.js
  /cli.js
    ✓ should show help with no args (234ms)
    ✓ should default to outputting to stdout (288ms)
    ✓ should support outputting to a file (--out) (257ms)
    ✓ should support outputting compositions to a file (--json) (258ms)
    ✓ should return the correct error code on invalid CSS (298ms)
    ✓ should support disabling url() rewriting (--no-rewrite) (295ms)

PASS packages/paths/test/paths.test.js
  modular-css-paths
    ✓ should return a falsey value if a file isn't found (3ms)
    ✓ should return the absolute path if a file is found (1ms)
    ✓ should check multiple paths for files & return the first match (1ms)
    ✓ should be usable as a modular-css resolver (30ms)

PASS packages/core/test/externals.test.js
  /processor.js
    externals
      ✓ should fail if not a valid composition reference (20ms)
      ✓ should fail if not referencing another file (4ms)
      ✓ should fail on bad class references (6ms)
      ✓ should support overriding external values (22ms)

PASS packages/namer/test/index.test.js
  modular-css-namer
    ✓ should hash its arguments (2ms)
    ✓ should differ within files (1ms)
    ✓ should re-use selectors for identical inputs
    ✓ should differ between files (1ms)
    ✓ should wrap as necessary (101ms)

PASS packages/core/test/exports.test.js
  /processor.js
    exports
      ✓ should export an object of arrays containing strings (18ms)
      ✓ should export identifiers and their classes (15ms)

PASS packages/glob/test/glob.test.js
  /glob.js
    ✓ should be a function (2ms)
    ✓ should use a default search (33ms)
    ✓ should find files on disk & output css (8ms)
    ✓ should support exclusion patterns (7ms)

PASS packages/core/test/getters.test.js
  /processor.js
    getters
      .file
        ✓ should return all the files that have been added (37ms)
      .options
        ✓ should return the merged options object

PASS packages/core/test/issues/issue-56.test.js
  /issues
    /56
      ✓ should prune rules that only compose, but leave them in the exports (25ms)

PASS packages/core/test/issues/issue-24.test.js
  /issues
    /24
      ✓ should be able to compose using a value (32ms)

PASS packages/core/test/issues/issue-98.test.js
  /issues
    /98
      ✓ should prune rules that only compose, but leave them in the exports (15ms)

PASS packages/core/test/unicode.test.js
  /processor.js
    unicode
      ✓ should support unicode classes & ids (35ms)

PASS packages/core/test/issues/issue-66.test.js
  /issues
    /66
      ✓ should ignore remove calls for unknown files (13ms)

PASS packages/core/test/issues/issue-261.test.js
  /issues
    /261
      ✓ should allow colons in rules that also use :external() (29ms)

Summary of all failing tests
FAIL packages/rollup/test/rollup.test.js
  ● /rollup.js › should handle assetFileNames being undefined

    ReferenceError: output is not defined

       97 |         await bundle.write({
       98 |             format,
    >  99 |             file : `${output}/assetFileNames/simple.js`,
          |                       ^
      100 |         });
      101 | 
      102 |         const [ css ] = shell.ls(`${output}/assetFileNames/assets`);

      at Object.it (packages/rollup/test/rollup.test.js:99:23)


Test Suites: 1 failed, 2 skipped, 29 passed, 30 of 32 total
Tests:       1 failed, 4 skipped, 198 passed, 203 total
Snapshots:   242 passed, 242 total
Time:        19.906s
Ran all test suites.
npm ERR! Test failed.  See above for more details.

@codecov
Copy link

codecov bot commented Jul 12, 2018

Codecov Report

Merging #449 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #449      +/-   ##
==========================================
+ Coverage   99.11%   99.12%   +<.01%     
==========================================
  Files          31       31              
  Lines         790      798       +8     
  Branches      121      123       +2     
==========================================
+ Hits          783      791       +8     
  Misses          7        7
Impacted Files Coverage Δ
packages/core/processor.js 100% <100%> (ø) ⬆️
packages/rollup/rollup.js 100% <100%> (ø) ⬆️
packages/browserify/browserify.js 97.82% <100%> (+0.07%) ⬆️

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 9e495dc...b3cec9f. Read the comment docs.

@tivac tivac merged commit d2eefec into master Jul 12, 2018
@tivac tivac deleted the rollup-rebuilds branch July 12, 2018 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants