[Feature Request] Import CSS using the package.json main attribute

[oleersoy]

Hi,

I'm creating pure css packages (superfly-css). The workflow I'm using looks like this:

1. Create new public github repository
2. Clone the repository
3. Run npm init
4. Add the style attribute to package.json
5. Publish to NPM

If postcss-import read the main attribute for the css file that the package publishes, it would allow me to skip step 4. Right now the main attribute duplicates the style attribute.

Thoughts?

TIA,
Ole

[zoubin]

If you want to publish a package with both js and css (like a react component), how do you specify the main field?

[oleersoy]

You can still use both. What I'm asking for is that the main field be checked first for a .css resource, then the style attribute.

[MoOx]

When I create a package that is only css, I were using and index.css.
But why not. Open for PR.

[oleersoy]

Cool - Just did a quick code scan. Doesn't look too scary :). Hope to have PR in the coming days. 👍

[MoOx]

https://github.com/postcss/postcss-import/blob/master/index.js#L498

Please add tests for this... I am not sure if current behavior is tested :(, but should be to be sure we are not breaking anything

[oleersoy]

OK - Will add tests 👍

[MoOx]

Cool, thanks!

[oleersoy]

Here's a revised more defensive version that has all the current tests passing:

      packageFilter: function processPackage(pkg) {
        if (pkg && pkg.main && pkg.main.indexOf('.css') == -1) {
          pkg.main = pkg.style || "index.css"
        }
        return pkg
      },

[MoOx]

You should start that .css is at the end of the string, to be sure you don't catch things like "stuff.css/lib/thing.js"

[oleersoy]

OK - Will make sure to look at end of String. Something strange is going on. Here's what I'm doing.

1. Clone the repository
2. Run the tests ( Pass)
3. Introduce the code as is currently (13 tests fail)
4. Change the t.plan parameter from 17 to 16
5. All the tests pass

Weird right?

[oleersoy]

Updated implementation:

      packageFilter: function processPackage(pkg) {
        if (pkg && pkg.main && pkg.main.endsWith('.css') == -1) {
          pkg.main = pkg.style || "index.css"
        }
        return pkg
      },

This passes the current tests with the exception that I have to change the t.plan argument (Line 35) from 17 to 16.

[MoOx]

If you are removing some tests, it means it's hiding an issue.

[oleersoy]

I'm not removing any tests...I don't think. I'm only changing the parameter. That's why I think it's strange. I'm following these precise steps:

1. Clone the repository
2. Run the tests
3. Change the implementation
4. Change 17 to 16 (Without removing a corresponding test)
5. Run the tests
6. Everything passes

This should not be the case, but it is....I tested it twice.

The reason I think that it should not be the case is that in step 2 when I run the tests, tape expects 17 test results and that's what it gets. Then I change the implementation. If I don't change 17 to 16, 13 tests fail. If I change 17 to 16, then everything passes. I'm only changing the numbers. Not removing any tests correspondingly. Seems like a tape bug...

[MoOx]

I will take a look when I have some times. Thanks for sharing the snippet.

[oleersoy]

Cool - thanks - It's my first time using tape, so I just want to make sure I'm doing the right thing. I've run it 4 times now. Same result. I'll start working on a test for the pkg.main case.

[oleersoy]

Another strange thing is happening.

1. Clone a fresh repository
2. Run npm run tape test (Tests pass)
3. Try changing test/fixtures/modules.css
4. Tests fail
   4.5) Restore test/fixtures/modules.css so that tests pass
5. Introduce the feature for using pkg.main
6. Run the tests (13 Tests Fail)
7. Change 17 to 16
8. Tests pass
9. Change test/fixtures/modules.css
10. Tests still pass

So it's like it's not even running the module test...Or just ignoring the results...

[oleersoy]

Getting a little closer to understanding what's going on. Here's why the change from 17 to 16 works:

BEFORE IMPLEMENTATION OF PKG.MAIN FEATURE
ok 14 should not need `path` option if `source` option has been passed
ok 15 should be able to consume npm package or local modules
ok 16 should not fail with only one absolute import
ok 17 should not fail with absolute and local import

AFTER IMPLEMENTATION OF PKG.MAIN FEATURE
ok 14 should not need `path` option if `source` option has been passed
ok 15 should not fail with only one absolute import
ok 16 should not fail with absolute and local import

So after the implementation this test:

ok 15 should be able to consume npm package or local modules

Stops running.

[oleersoy]

I left the above trail just in case there is a bug in both tape and postcss-import. I have narrowed it down to this code block (Lines 532 - 540):

  } catch (e) {
    var message = "Failed to find '" + name + "' from " + root +
      "\n    in [ " +
      "\n        " + paths.join(",\n        ") +
      "\n    ]"
    console.log("ERROR: ", message);

    throw new Error(message, source)
  }

I'm only running the modules test. Here's the message:

TAP version 13
# @import
ERROR:  Failed to find 'fake' from /home/ole/Sandbox/temp2/postcss-import/test
    in [ 
        /home/ole/Sandbox/temp2/postcss-import/test/fixtures/imports
    ]
not ok 1 plan != count
  ---
    operator: fail
    expected: 1
    actual:   0
  ...

1..1
# tests 1
# pass  0
# fail  1

Thoughts on how to fix?

[oleersoy]

Here's the trimmed down test. Note that I modified the catch block above to use logging.

var test = require("tape")

var assign = require("object-assign")
var path = require("path")
var fs = require("fs")

var atImport = require("..")
var postcss = require("postcss")
var fixturesDir = path.join(__dirname, "fixtures")
var importsDir = path.join(fixturesDir, "imports")

function read(name) {
  return fs.readFileSync("test/" + name + ".css", "utf8").trim()
}

function compareFixtures(t, name, msg, opts, postcssOpts) {
  opts = assign({
    path: importsDir
  }, opts || {})
  postcss(atImport(opts))
    .process(read("fixtures/" + name), postcssOpts)
    .then(trimResultCss)
    .then(function(actual) {
      var expected = read("fixtures/" + name + ".expected")
      // handy thing: checkout actual in the *.actual.css file
      fs.writeFile("test/fixtures/" + name + ".actual.css", actual)
      t.equal(actual, expected, msg)
    })
}

function trimResultCss(result) {
  return result.css.trim()
}

test("@import", function(t) {
  t.plan(1)
  compareFixtures(
    t,
    "modules",
    "should be able to consume npm package or local modules",
    {
      root: __dirname
    }
  )
})

[RobLoach]

This is looking great, @TrySound 👍

[oleersoy]

Hi,

Finally got time to test this out. I updated superfly-css-task-build to use postcss-import version ^9.1.0. This is the task that uses postcss-import to import pure css modules ... hopefully using the main attribute.

Then I removed the style attribute from the package.json file of superfly-css-variables-colors and bumped the version.

I'm now attempting to build superfly-css-utilities-colors like this:

git clone https://github.com/superfly-css/superfly-css-utilities-colors
npm install
gulp build:css

However I get the lint message:

22:1	⚠  Unable to find uri in '@import superfly-css-variables-colors' [postcss-import]

Thoughts?

TIA,
Ole

[RyanZim]

@oleersoy You have to double-quote the package name: @import "superfly-css-variables-colors"

[oleersoy]

@RyanZim thanks for the heads up! It seems to be working fine now, except for some strange logging that's going on. For example suppose If I delete everything from src/main/css/index.css and only leave:

@import "superfly-css-variables-colors";

The following console out results from the build:

ole@MKI:~/superfly-css-utilities-colors$ gulp build:css
[12:28:58] Using gulpfile ~/Junk/superfly-css-utilities-colors/gulpfile.js
[12:28:58] Starting 'build:css'...
[12:28:58] Finished 'build:css' after 16 ms

src/main/css/index.css
undefined [undefined]

Any idea what's causing the:

src/main/css/index.css
undefined [undefined]

part?

[oleersoy]

OK - The postcss-reporter plugin is causing it ... I'll follow it up with them. Thanks again.