Conversation
Codecov Report
@@ Coverage Diff @@
## master #135 +/- ##
==========================================
+ Coverage 97.35% 97.63% +0.28%
==========================================
Files 2 2
Lines 151 169 +18
==========================================
+ Hits 147 165 +18
Misses 4 4
Continue to review full report at Codecov.
|
|
As I mentioned in #136, this is failing with Note: I have a pull request at stackcss/postcss-prefix#13 that should fix this. |
yoshuawuyts
force-pushed
the
use-transform
branch
from
91d6455 to
dda7da3
Compare
|
@cjhowe7 could you maybs give this another review? Thanks! 😁 |
cjhowe-us
left a comment
cjhowe-us
left a comment
There was a problem hiding this comment.
Woops, looks like I made a mistake on one of the tests. I'll make a PR to fix it. There's a few lines of unnecessary code I could remove too.
| ] | ||
| }, | ||
| "dependencies": { | ||
| "sheetify": "file:../../..", |
There was a problem hiding this comment.
Not so sure these dependencies are necessary.
There was a problem hiding this comment.
Confirmed they are unnecessary. I will PR shortly to remove this.
test/transform-package.js
Outdated
| t.equal(res, expected, 'CSS was transformed') | ||
| }) | ||
|
|
||
| const bOpts = { browserField: false } |
| next() | ||
| }) | ||
| }) | ||
| .plugin('css-extract', { out: outFn }) |
There was a problem hiding this comment.
Well, this is definitely not supposed to be here for this test. Woops.
There was a problem hiding this comment.
No, wait, my bad. This is fine. I guess I confused this with sheetify-cssnext.
|
Thanks!y
…On Mon, Oct 2, 2017 at 12:50 PM Christian Howe ***@***.***> wrote:
***@***.**** commented on this pull request.
Woops, looks like I made a mistake on one of the tests. I'll make a PR to
fix it. There's a few lines of unnecessary code I could remove too.
------------------------------
In test/fixtures/transform-package/package.json
<#135 (comment)>:
> @@ -0,0 +1,16 @@
+{
+ "sheetify": {
+ "transform": [
+ [
+ "sheetify-cssnext",
+ {
+ "sourcemap": false
+ }
+ ]
+ ]
+ },
+ "dependencies": {
+ "sheetify": "file:../../..",
Not so sure these dependencies are necessary.
------------------------------
In test/transform-package.js
<#135 (comment)>:
> +
+const sheetify = require('../')
+
+test('transform-package', function (t) {
+ t.test('should transform CSS from package.json with options', function (t) {
+ t.plan(1)
+
+ const expath = path.join(__dirname, 'fixtures/transform-package/expected.css')
+ const expected = fs.readFileSync(expath, 'utf8').trim()
+
+ const ws = concat(function (buf) {
+ const res = String(buf).trim()
+ t.equal(res, expected, 'CSS was transformed')
+ })
+
+ const bOpts = { browserField: false }
This is probably unnecessary
------------------------------
In test/transform-package.js
<#135 (comment)>:
> + t.equal(res, expected, 'CSS was transformed')
+ })
+
+ const bOpts = { browserField: false }
+ const bpath = path.join(__dirname, 'fixtures/transform-package/source.js')
+
+ browserify(bpath, bOpts)
+ .transform(sheetify)
+ .transform(function (file) {
+ return through(function (buf, enc, next) {
+ const str = buf.toString('utf8')
+ this.push(str.replace(/sheetify\/insert/g, 'insert-css'))
+ next()
+ })
+ })
+ .plugin('css-extract', { out: outFn })
Well, this is definitely not supposed to be here for this test. Woops.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#135 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACWlelnZJaLonU07A6kM9T-nb7TNuqrcks5soRRvgaJpZM4PfMxO>
.
|
|
I must have been tired when I did that review, so it should actually be working fine. If you want, I opened a pull request (#137) to remove some unnecessary options from the tests to keep them more minimal. |
|
|
3 participants
Semver major. Replaces
--usewith--transform. Tests pass, but not sure ifpackage.jsondetection works. Did we ever get that to work? Thanks!