feat: support from global for composes #669

jquense · 2019-09-13T19:18:35Z

This is nice feature in css-modules, for doing quick composition of global classes. global acts as a keyword instead of a file here

Description

Parses composes: a, b, c from global as composes: global(a), global(b), global(c)

Motivation and Context

Css-modules supports this, and it is nicer syntax when working with lots of global classes, such with bootstrap utility classes

How Has This Been Tested?

added a test. I could get the current tests to pass locally tho, perhaps i'm setting the repo up wrong? did lerna bootstrap in the root, assuming that was enough.

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.

This is nice feature in css-modules, for doing quick composition of global classes. `global` acts as a keyword instead of a file here

TravisBuddy · 2019-09-13T19:21:18Z

Travis tests have failed

Hey @jquense,
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 parsers


> modular-css@0.0.0 parsers /home/travis/build/tivac/modular-css
> node parsers/build.js


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

 PASS   tests  packages/rollup/test/rollup.test.js
 PASS   tests  packages/processor/test/options.test.js
 PASS   tests  packages/processor/test/api.test.js
 PASS   tests  packages/rollup/test/watch.test.js
 PASS   tests  packages/rollup/test/splitting.test.js
 PASS   tests  packages/svelte/test/svelte.test.js
(node:5494) DeprecationWarning: Tapable.plugin is deprecated. Use new API on `.hooks` instead
 PASS   tests  packages/webpack/test/webpack.test.js
 PASS   tests  packages/svelte/test/rollup.test.js
 PASS   tests  packages/browserify/test/factor-bundle.test.js
 PASS   tests  packages/rollup-rewriter/test/rewriter.test.js
 PASS   tests  packages/browserify/test/browserify.test.js
 FAIL   tests  packages/processor/test/composition.test.js
  ● /processor.js › composition › should fail on invalid composes syntax

    expect(received).toMatch(expected)

    Expected substring: "SyntaxError: Expected source but \"n\" found."
    Received string:    "modular-css-graph-nodes: /home/travis/build/tivac/modular-css/invalid/value.css:1:23: SyntaxError: Expected \"global\" or source but \"n\" found."

      23 |                 );
      24 |             } catch({ message }) {
    > 25 |                 expect(message).toMatch(`SyntaxError: Expected source but "n" found.`);
         |                                 ^
      26 |             }
      27 |         });
      28 | 

      at Object.toMatch (packages/processor/test/composition.test.js:25:33)

 PASS   tests  packages/processor/test/values.test.js
 PASS   tests  packages/postcss/test/postcss.test.js
 PASS   tests  packages/browserify/test/watchify.test.js
 PASS   tests  packages/processor/test/at-composes.test.js
 PASS   tests  packages/aliases/test/aliases.test.js
 PASS   tests  packages/processor/test/scoping.test.js
 PASS   tests  packages/processor/test/keyframes.test.js
 PASS   tests  packages/browserify/test/issue-58.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/getters.test.js
 PASS   tests  packages/processor/test/externals.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/processor/test/composition.test.js
  ● /processor.js › composition › should fail on invalid composes syntax

    expect(received).toMatch(expected)

    Expected substring: "SyntaxError: Expected source but \"n\" found."
    Received string:    "modular-css-graph-nodes: /home/travis/build/tivac/modular-css/invalid/value.css:1:23: SyntaxError: Expected \"global\" or source but \"n\" found."

      23 |                 );
      24 |             } catch({ message }) {
    > 25 |                 expect(message).toMatch(`SyntaxError: Expected source but "n" found.`);
         |                                 ^
      26 |             }
      27 |         });
      28 | 

      at Object.toMatch (packages/processor/test/composition.test.js:25:33)


Test Suites: 1 failed, 1 skipped, 33 passed, 34 of 35 total
Tests:       1 failed, 5 skipped, 287 passed, 293 total
Snapshots:   324 passed, 324 total
Time:        25.166s
Ran all test suites.
npm ERR! Test failed.  See above for more details.

TravisBuddy Request Identifier: a4d34cf0-d65b-11e9-82ee-13bec588d4b8

codecov · 2019-09-13T19:32:52Z

Codecov Report

Merging #669 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #669   +/-   ##
=======================================
  Coverage   99.12%   99.12%           
=======================================
  Files          45       45           
  Lines        1139     1139           
  Branches      174      174           
=======================================
  Hits         1129     1129           
  Misses         10       10

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 a57cddf...1bccc36. Read the comment docs.

tivac

Looks great, and I'm very pleased at how straightforward the changes were 😅

You've got the checkboxes all set up correctly marking that this still needs docs, which should just be a a new example and some small updates to packages/www/src/guide/feature-composing-styles.md

parsers/shared.pegjs

parsers/composes.pegjs

tivac · 2019-09-13T22:13:13Z

packages/processor/test/composition.test.js

@@ -22,10 +22,10 @@ describe("/processor.js", () => {
                    ".a { composes: b from nowhere.css; }"
                );
            } catch({ message }) {
-                expect(message).toMatch(`SyntaxError: Expected source but "n" found.`);
+                expect(message).toMatch(`SyntaxError: Expected global or source but "n" found.`);


Ugh, I wish it was easier in pegjs to customize the error messages. Nothing you need to change here, just annoyed and sharing.

jquense · 2019-09-14T15:29:19Z

I was really impressed with how easy this was to do. I initially saw the generated parsers and thought it was a bit overkill, but i'm convinced now, gonna steal this strategy for other stuff!

jquense · 2019-09-14T15:29:53Z

packages/www/src/guide/feature-composing-styles.md

-    local    : "dafdfcc_other aeacf0c_single"
+    composable : "dafdfcc_composable",
+    local      : "dafdfcc_composable aeacf0c_local",
+    removed    : "dafdfcc_composable aeacf0c_local aeacf0c_removed"


I updated this to match the output of the example file above

TravisBuddy · 2019-09-15T22:50:00Z

Hey @jquense,
Something went wrong with the build.

TravisCI finished with status errored, which means the build failed because of something unrelated to the tests, such as a problem with a dependency or the build process itself.

View build log

TravisBuddy Request Identifier: 249a90a0-d80b-11e9-b22f-8dc32e2a8e5f

TravisBuddy · 2019-09-16T00:10:55Z

Hey @jquense,
Something went wrong with the build.

TravisCI finished with status errored, which means the build failed because of something unrelated to the tests, such as a problem with a dependency or the build process itself.

View build log

TravisBuddy Request Identifier: 728d8c30-d816-11e9-94f2-eb1f1b783936

tivac · 2019-09-16T06:55:49Z

Released in v25.0.0, thanks @jquense!

feat: support from global for composes

57cae41

This is nice feature in css-modules, for doing quick composition of global classes. `global` acts as a keyword instead of a file here

nicer error

8d8585c

tivac requested changes Sep 13, 2019

View reviewed changes

address feedback

a642bee

jquense commented Sep 14, 2019

View reviewed changes

docs: fix small syntax bit

1bccc36

tivac approved these changes Sep 15, 2019

View reviewed changes

tivac merged commit 0a7996e into tivac:master Sep 15, 2019

jquense deleted the from-global branch September 16, 2019 12:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support from global for composes #669

feat: support from global for composes #669

jquense commented Sep 13, 2019 •

edited

Loading

TravisBuddy commented Sep 13, 2019

codecov bot commented Sep 13, 2019 •

edited

Loading

tivac left a comment

tivac Sep 13, 2019

jquense commented Sep 14, 2019 •

edited

Loading

jquense Sep 14, 2019

TravisBuddy commented Sep 15, 2019

TravisBuddy commented Sep 16, 2019

tivac commented Sep 16, 2019

feat: support from global for composes #669

feat: support from global for composes #669

Conversation

jquense commented Sep 13, 2019 • edited Loading

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

TravisBuddy commented Sep 13, 2019

Travis tests have failed

Node.js: 8

TravisBuddy Request Identifier: a4d34cf0-d65b-11e9-82ee-13bec588d4b8

codecov bot commented Sep 13, 2019 • edited Loading

Codecov Report

tivac left a comment

Choose a reason for hiding this comment

tivac Sep 13, 2019

Choose a reason for hiding this comment

jquense commented Sep 14, 2019 • edited Loading

jquense Sep 14, 2019

Choose a reason for hiding this comment

TravisBuddy commented Sep 15, 2019

TravisBuddy Request Identifier: 249a90a0-d80b-11e9-b22f-8dc32e2a8e5f

TravisBuddy commented Sep 16, 2019

TravisBuddy Request Identifier: 728d8c30-d816-11e9-94f2-eb1f1b783936

tivac commented Sep 16, 2019

jquense commented Sep 13, 2019 •

edited

Loading

codecov bot commented Sep 13, 2019 •

edited

Loading

jquense commented Sep 14, 2019 •

edited

Loading