-
Notifications
You must be signed in to change notification settings - Fork 115
Add cache #47
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
Add cache #47
Changes from 4 commits
fe1a753
f183e59
190df56
c8c3bbd
b69f080
177ef2e
466ce1f
3642fff
389121c
b1a48c5
4bc907b
936a93e
d0b054c
20bced0
00cac1b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,3 @@ | ||
| node_modules | ||
| test/fixtures/*.actual.css | ||
| test/cache |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,13 +10,15 @@ var postcss = require("postcss") | |
|
|
||
| var fixturesDir = path.join(__dirname, "fixtures") | ||
| var importsDir = path.join(fixturesDir, "imports") | ||
| var cacheDir = path.join(__dirname, "cache") | ||
|
|
||
| function read(name) { | ||
| return fs.readFileSync("test/" + name + ".css", "utf8").trim() | ||
| } | ||
|
|
||
| function compareFixtures(t, name, msg, opts, postcssOpts) { | ||
| opts = opts || {path: importsDir} | ||
| postcssOpts = postcssOpts || {} | ||
| var actual = postcss() | ||
| .use(atImport(opts)) | ||
| .process(read("fixtures/" + name), postcssOpts) | ||
|
|
@@ -190,3 +192,27 @@ test("works with no styles at all", function(t) { | |
|
|
||
| t.end() | ||
| }) | ||
|
|
||
| test("works with caching", function(t) { | ||
| var opts = {path: importsDir, cacheDir: cacheDir} | ||
| var cssFileName = path.resolve("test/fixtures/relative-to-source.css") | ||
| var postcssOpts = {from: cssFileName} | ||
| var css = fs.readFileSync(cssFileName) | ||
| var output = postcss() | ||
| .use(atImport(opts)) | ||
| .process(css, postcssOpts) | ||
| .css.trim() | ||
|
|
||
| var imports = JSON.parse(fs.readFileSync(cacheDir + "/imports.json")) | ||
| var cacheFileName = imports[cssFileName].cache; | ||
| var cache = fs.readFileSync(cacheFileName, "utf8").trim() | ||
|
|
||
| t.equal(output, cache, "should put output in cache") | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we know here that the cache is written, how do we know the cache is being read after that ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm sorry @MoOx, there's no way of knowing. It's a transparent cache. Unless you're suggesting that we should add a bunch of debug code to poll the internal state of the engine, which I would strongly advice against, we might as well close the PR because there's no way for me to satisfy this need. I know that the cache is hit by using breakpoints, I know that it returns a correct and consistent result, and I know that I get a 100ms performance increase when using it with a local project of mine, but there's no way for me to put that into a unit test. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is always a way. What about adding a simple map object with {file-requested: bool flag} to know if a requested file as been used from cache? Doesn't seems hard to do and this won't affect perfs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't quite understand what you mean, but you're welcome to take a stab at it. |
||
|
|
||
| output = postcss() | ||
| .use(atImport(opts)) | ||
| .process(css, postcssOpts) | ||
| .css.trim() | ||
| t.equal(output, cache, "should return identical result on subsequent calls") | ||
| t.end() | ||
| }) | ||
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so you are not caching import that use a media query ? why ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Media queried imports will be cached by the requesting file. One shouldn't cache the file when requested with a media query because it will alter the output from how it looks when requested normally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hum... ok :)