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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: force rollup output to use .css #563

Merged
merged 4 commits into from Feb 6, 2019

Conversation

Projects
None yet
2 participants
@tivac
Copy link
Owner

commented Feb 6, 2019

Description

Don't use the source file's original extension, it may not be .css and that does bad things.

See https://5c5a73dabd66690008908a73--m-css.netlify.com/ for an example of the "bad things" I'm referring to 馃え

Motivation and Context

In this case it was the svelte plugin's support for inline <style> tags, meaning that a bunch of .html files were generated containing CSS. Most web servers handle content-type by file extension, so this went poorly.

How Has This Been Tested?

Added a test, also ran https://m-css.com locally & verified that this fixed the issue.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@tivac tivac self-assigned this Feb 6, 2019

@TravisBuddy

This comment has been minimized.

Copy link

commented Feb 6, 2019

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

View build log

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


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


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

PASS lint packages/rollup/test/rollup.test.js
  鈼 Console

    console.warn 
      
      /home/travis/build/tivac/modular-css/packages/rollup/test/rollup.test.js
        663:9  warning  Skipped test  jest/no-disabled-tests
      
      鉁 1 problem (0 errors, 1 warning)
      

PASS lint packages/processor/test/options.test.js
PASS lint packages/processor/test/api.test.js
PASS lint packages/rollup/test/splitting.test.js
PASS lint packages/processor/processor.js
PASS lint packages/rollup/test/watch.test.js
  鈼 Console

    console.warn 
      
      /home/travis/build/tivac/modular-css/packages/rollup/test/watch.test.js
        7:7  warning  'read' is assigned a value but never used  no-unused-vars
      
      鉁 1 problem (0 errors, 1 warning)
      

PASS lint packages/rollup/rollup.js
PASS lint packages/svelte/test/svelte.test.js
PASS lint packages/webpack/test/webpack.test.js
PASS lint packages/browserify/browserify.js
PASS lint packages/browserify/test/factor-bundle.test.js
PASS lint packages/svelte/svelte.js
PASS lint packages/browserify/test/browserify.test.js
PASS lint packages/processor/test/composition.test.js
PASS lint packages/processor/test/values.test.js
PASS lint packages/rollup-rewriter/test/rewriter.test.js
PASS lint packages/www/src/repl/store.js
  鈼 Console

    console.warn 
      
      /home/travis/build/tivac/modular-css/packages/www/src/repl/store.js
         97:13  warning  Unexpected console statement  no-console
        117:13  warning  Unexpected console statement  no-console
      
      鉁 2 problems (0 errors, 2 warnings)
      

PASS lint packages/www/rollup.config.js
PASS lint packages/postcss/test/postcss.test.js
PASS lint packages/browserify/test/watchify.test.js
PASS lint packages/rollup-rewriter/rewriter.js
  鈼 Console

    console.warn 
      
      /home/travis/build/tivac/modular-css/packages/rollup-rewriter/rewriter.js
        46:34  warning  'imports' is assigned a value but never used  no-unused-vars
      
      鉁 1 problem (0 errors, 1 warning)
      

PASS lint packages/rollup/chunker.js
PASS lint packages/scratchpad/chunks.js
PASS lint packages/processor/plugins/composition.js
PASS lint packages/processor/plugins/scoping.js
PASS lint packages/processor/plugins/values-replace.js
PASS lint packages/webpack/plugin.js
PASS lint packages/aliases/test/aliases.test.js
PASS lint packages/processor/test/scoping.test.js
PASS lint packages/browserify/test/issue-58.test.js
PASS lint packages/processor/test/keyframes.test.js
PASS lint packages/browserify/test/issue-313.test.js
PASS lint packages/cli/test/cli.test.js
PASS lint packages/paths/test/paths.test.js
PASS lint packages/processor/plugins/externals.js
PASS lint packages/processor/test/externals.test.js
PASS lint packages/processor/plugins/graph-nodes.js
PASS lint packages/webpack/loader.js
PASS lint packages/cli/cli.js
PASS lint packages/browserify/test/issue-105.test.js
  鈼 Console

    console.warn 
      
      /home/travis/build/tivac/modular-css/packages/browserify/test/issue-105.test.js
        16:13  warning  Skipped test  jest/no-disabled-tests
        23:13  warning  Skipped test  jest/no-disabled-tests
      
      鉁 2 problems (0 errors, 2 warnings)
      

PASS lint packages/processor/test/getters.test.js
PASS lint packages/scratchpad/test/chunks.test.js
PASS lint packages/www/build/html/guide.js
PASS lint packages/www/build/rollup-plugin-postcss.js
PASS lint packages/namer/test/namer.test.js
PASS lint packages/www/build/html/repl.js
PASS lint packages/glob/test/glob.test.js
PASS lint packages/postcss/postcss.js
PASS lint packages/www/build/rollup-plugin-generate-html.js
PASS lint packages/processor/test/exports.test.js
PASS lint packages/namer/namer.js
PASS lint packages/www/build/html/changelog.js
PASS lint packages/processor/test/issues/issue-191.test.js
PASS lint packages/www/build/html/home.js
PASS lint packages/processor/test/issues/issue-56.test.js
PASS lint packages/processor/test/issues/issue-24.test.js
PASS lint packages/www/build/html/css.js
PASS lint packages/processor/test/issues/issue-98.test.js
PASS lint packages/processor/plugins/values-local.js
PASS lint packages/processor/plugins/values-composed.js
PASS lint packages/processor/plugins/values-namespaced.js
PASS lint packages/processor/plugins/values-imported.js
PASS lint packages/www/build/svelte.js
PASS lint packages/scratchpad/chunktest.js
  鈼 Console

    console.warn 
      
      /home/travis/build/tivac/modular-css/packages/scratchpad/chunktest.js
        37:1   warning  Unexpected console statement  no-console
        40:5   warning  Unexpected console statement  no-console
        43:20  warning  Unexpected console statement  no-console
        46:1   warning  Unexpected console statement  no-console
      
      鉁 4 problems (0 errors, 4 warnings)
      

PASS lint packages/processor/plugins/values-export.js
PASS lint packages/scratchpad/test/construct.js
PASS lint packages/www/build/rollup-plugin-sirv.js
PASS lint packages/processor/plugins/keyframes.js
PASS lint packages/rollup-rewriter/formats/amd.js
PASS lint packages/processor/test/unicode.test.js
PASS lint packages/aliases/aliases.js
PASS lint packages/processor/lib/graph-tiers.js
PASS lint packages/rollup-rewriter/formats/system.js
PASS lint packages/glob/glob.js
PASS lint packages/test-utils/logs.js
PASS lint packages/processor/lib/resolve.js
PASS lint packages/processor/lib/output.js
PASS lint packages/test-utils/rollup-build-snapshot.js
PASS lint packages/processor/test/issues/issue-66.test.js
PASS lint packages/processor/test/issues/issue-261.test.js
PASS lint packages/www/src/repl/listen.js
PASS lint packages/www/build/rollup-plugin-add-watch-files.js
PASS lint packages/test-utils/rollup-code-snapshot.js
PASS lint packages/test-utils/read-dir.js
PASS lint packages/processor/lib/identifiers.js
PASS lint packages/paths/paths.js
PASS lint packages/browserify/test/lib/bundle.js
PASS lint packages/scratchpad/test/snapshot.js
PASS lint packages/processor/lib/normalize.js
PASS lint packages/rollup-rewriter/formats/es.js
PASS lint packages/test-utils/relative.js
PASS lint packages/test-utils/write.js
PASS lint packages/test-utils/rollup-watching.js
PASS lint packages/www/src/repl/data/prompt.js
PASS lint packages/processor/lib/message.js
PASS lint packages/www/build/rollup-plugin-clean.js
PASS lint packages/www/build/environment.js
PASS lint packages/processor/lib/relative.js
PASS lint packages/www/stubs/path.js
PASS lint packages/www/build/html/markdown.js
PASS lint packages/test-utils/read.js
PASS lint packages/www/src/repl/codemirror.js
PASS lint packages/test-utils/exists.js
PASS lint packages/test-utils/warn.js
PASS lint packages/test-utils/prefix.js
PASS lint packages/test-utils/namer.js
PASS lint packages/www/stubs/fs.js
PASS lint packages/www/src/repl/index.js
FAIL tests packages/rollup/test/rollup.test.js
  鈼 /rollup.js 鈥 should output assets with a .css file extension

    expect(value).toMatchSnapshot()

    Received value does not match stored snapshot "/rollup.js should output assets with a .css file extension 1".

    - Snapshot
    + Received

    @@ -1,7 +1,7 @@
      Object {
    -   "assets/style-ea90c2f3.css": "
    +   "assets/style-364a3e62.css": "
      /* packages/rollup/test/specimens/file-extension/style.cssx */
      .style {
          color: salmon;
      }
      ",

      590 |         const result = await bundle.generate({ format });
      591 | 
    > 592 |         expect(result).toMatchRollupSnapshot();
          |                        ^
      593 |     });
      594 | 
      595 |     describe("case sensitivity tests", () => {

      at Object.toMatchRollupSnapshot (packages/rollup/test/rollup.test.js:592:24)

 鈥 1 snapshot failed.
PASS tests packages/processor/test/options.test.js
PASS tests packages/processor/test/api.test.js
PASS tests packages/rollup/test/splitting.test.js
PASS tests packages/rollup/test/watch.test.js
PASS tests packages/svelte/test/svelte.test.js
(node:5275) DeprecationWarning: Tapable.plugin is deprecated. Use new API on `.hooks` instead
PASS tests packages/webpack/test/webpack.test.js
PASS tests packages/browserify/test/factor-bundle.test.js
PASS tests packages/browserify/test/browserify.test.js
PASS tests packages/processor/test/composition.test.js
PASS tests packages/processor/test/values.test.js
PASS tests packages/rollup-rewriter/test/rewriter.test.js
PASS tests packages/postcss/test/postcss.test.js
PASS tests packages/browserify/test/watchify.test.js
PASS tests packages/aliases/test/aliases.test.js
PASS tests packages/processor/test/scoping.test.js
PASS tests packages/browserify/test/issue-58.test.js
PASS tests packages/processor/test/keyframes.test.js
PASS tests packages/browserify/test/issue-313.test.js
PASS tests packages/cli/test/cli.test.js
PASS tests packages/paths/test/paths.test.js
PASS tests packages/processor/test/externals.test.js
PASS tests packages/processor/test/getters.test.js
PASS tests packages/scratchpad/test/chunks.test.js
PASS tests packages/namer/test/namer.test.js
PASS tests packages/glob/test/glob.test.js
PASS tests packages/processor/test/exports.test.js
PASS tests packages/processor/test/issues/issue-56.test.js
PASS tests packages/processor/test/issues/issue-24.test.js
PASS tests packages/processor/test/issues/issue-98.test.js
PASS tests packages/processor/test/unicode.test.js
PASS tests packages/processor/test/issues/issue-66.test.js
PASS tests packages/processor/test/issues/issue-261.test.js

Summary of all failing tests
FAIL packages/rollup/test/rollup.test.js
  鈼 /rollup.js 鈥 should output assets with a .css file extension

    expect(value).toMatchSnapshot()

    Received value does not match stored snapshot "/rollup.js should output assets with a .css file extension 1".

    - Snapshot
    + Received

    @@ -1,7 +1,7 @@
      Object {
    -   "assets/style-ea90c2f3.css": "
    +   "assets/style-364a3e62.css": "
      /* packages/rollup/test/specimens/file-extension/style.cssx */
      .style {
          color: salmon;
      }
      ",

      590 |         const result = await bundle.generate({ format });
      591 | 
    > 592 |         expect(result).toMatchRollupSnapshot();
          |                        ^
      593 |     });
      594 | 
      595 |     describe("case sensitivity tests", () => {

      at Object.toMatchRollupSnapshot (packages/rollup/test/rollup.test.js:592:24)


Snapshot Summary
 鈥 1 snapshot failed from 1 test suite. Inspect your code changes or run `npm test -- -u` to update them.

Test Suites: 1 failed, 71 skipped, 140 passed, 141 of 212 total
Tests:       1 failed, 74 skipped, 369 passed, 444 total
Snapshots:   1 failed, 308 passed, 309 total
Time:        26.388s
Ran all test suites in 2 projects.
npm ERR! Test failed.  See above for more details.
TravisBuddy Request Identifier: 2201edd0-29d7-11e9-9562-95bccb7073f8
test: remove hash
It's not stable cross-platform? Weird...
@codecov

This comment has been minimized.

Copy link

commented Feb 6, 2019

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #563      +/-   ##
==========================================
+ Coverage   99.06%   99.06%   +<.01%     
==========================================
  Files          49       49              
  Lines        1172     1173       +1     
  Branches      183      183              
==========================================
+ Hits         1161     1162       +1     
  Misses         11       11
Impacted Files Coverage 螖
packages/rollup/rollup.js 100% <100%> (酶) 猬嗭笍

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 4c0bdd8...2f92244. Read the comment docs.

@tivac tivac merged commit 8f4348e into master Feb 6, 2019

6 of 9 checks passed

Header rules No header rules processed
Details
Pages changed All files already uploaded
Details
Redirect rules No redirect rules processed
Details
Mixed content No mixed content detected
Details
codecov/patch 100% of diff hit (target 99.06%)
Details
codecov/project 99.06% (+<.01%) compared to 4c0bdd8
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details

@tivac tivac deleted the rollup-extensions branch Feb 6, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.