Skip to content
Permalink
Browse files

feat: processor.normalize() & corrected chunking logic (#562)

* fix: functional CSS chunking algorithm

Walks JS entries + CSS dep graph to determine ideal chunking locations and merges nodes together until it gets there. Probably a bit slower than before but *way* more accurate and safer to use.

* fix: change asset naming to not depend on entries

There were just too many issues around trying to parse out the source filename, so for now we use the CSS files instead.

* feat: processor.normalize()

Because sometimes you need external access to cleaned-up paths

Fixes #559 
Fixes #560
  • Loading branch information...
tivac committed Feb 5, 2019
1 parent 23569dc commit e0c5eeefc61f669655f304781536909945094dba
Showing with 1,210 additions and 553 deletions.
  1. +6 −6 packages/namer/namer.js
  2. +21 −8 packages/processor/processor.js
  3. +6 −0 packages/processor/test/__snapshots__/api.test.js.snap
  4. +27 −1 packages/processor/test/api.test.js
  5. +11 −0 packages/processor/test/getters.test.js
  6. +35 −13 packages/rollup-rewriter/rewriter.js
  7. +40 −40 packages/rollup-rewriter/test/__snapshots__/rewriter.test.js.snap
  8. +136 −0 packages/rollup/chunker.js
  9. +1 −4 packages/rollup/package.json
  10. +0 −32 packages/rollup/parser.js
  11. +70 −126 packages/rollup/rollup.js
  12. +30 −30 packages/rollup/test/__snapshots__/rollup.test.js.snap
  13. +129 −153 packages/rollup/test/__snapshots__/splitting.test.js.snap
  14. +156 −66 packages/rollup/test/__snapshots__/watch.test.js.snap
  15. +2 −3 packages/rollup/test/rollup.test.js
  16. +5 −0 packages/rollup/test/specimens/css-dependencies/a.css
  17. +3 −0 packages/rollup/test/specimens/css-dependencies/a.js
  18. +5 −0 packages/rollup/test/specimens/css-dependencies/b.css
  19. +3 −0 packages/rollup/test/specimens/css-dependencies/b.js
  20. +3 −0 packages/rollup/test/specimens/css-dependencies/c.css
  21. +2 −2 packages/rollup/test/specimens/multiple-chunks/a.css
  22. +1 −10 packages/rollup/test/specimens/multiple-chunks/a.js
  23. +1 −1 packages/rollup/test/specimens/multiple-chunks/b.css
  24. +1 −10 packages/rollup/test/specimens/multiple-chunks/b.js
  25. +0 −5 packages/rollup/test/specimens/multiple-chunks/c.css
  26. +0 −4 packages/rollup/test/specimens/multiple-chunks/c.js
  27. +0 −1 packages/rollup/test/specimens/multiple-chunks/common.js
  28. +0 −1 packages/rollup/test/specimens/multiple-chunks/constants.css
  29. +0 −5 packages/rollup/test/specimens/multiple-chunks/d.css
  30. +0 −4 packages/rollup/test/specimens/multiple-chunks/d.js
  31. +0 −1 packages/rollup/test/specimens/multiple-chunks/e.js
  32. +0 −5 packages/rollup/test/specimens/multiple-chunks/shared.css
  33. +1 −0 packages/rollup/test/specimens/multiple-chunks/subfolder/a.css
  34. +29 −0 packages/rollup/test/splitting.test.js
  35. +16 −22 packages/rollup/test/watch.test.js
  36. +5 −0 packages/scratchpad/.npmignore
  37. +5 −0 packages/scratchpad/CHANGELOG.md
  38. +21 −0 packages/scratchpad/LICENSE
  39. +135 −0 packages/scratchpad/chunks.js
  40. +46 −0 packages/scratchpad/chunktest.js
  41. +10 −0 packages/scratchpad/package.json
  42. +135 −0 packages/scratchpad/test/__snapshots__/chunks.test.js.snap
  43. +60 −0 packages/scratchpad/test/chunks.test.js
  44. +35 −0 packages/scratchpad/test/construct.js
  45. +18 −0 packages/scratchpad/test/snapshot.js
@@ -36,13 +36,13 @@ module.exports = () => {

const { id, selectors } = meta.get(file);

// Has to use "in" because they can be 0 which is falsey
if(!selectors.has(selector)) {
selectors.set(selector, selectors.size);
}
// Don't need to cache this because repeats were caught by the cache up above
selectors.set(selector, selectors.size);

const output = value(letters, id) + value(everything, selectors.get(selector));

cache[key] = value(letters, id) + value(everything, selectors.get(selector));
cache.set(key, output);

return cache[key];
return output;
};
};
@@ -145,6 +145,11 @@ class Processor {
return files;
}

// Return the corrected-path version of the file
normalize(file) {
return this._normalize(file);
}

// Check if a file exists in the currently-processed set
has(input) {
const file = this._normalize(input);
@@ -167,7 +172,7 @@ class Processor {
}

const deps = this.dependents(source);

deps.concat(source).forEach((file) => {
this._log("invalidate()", file);

@@ -176,29 +181,33 @@ class Processor {
}

// Get the dependency order for a file or the entire tree
dependencies(file) {
dependencies(file, options = false) {
const { leavesOnly } = options;

if(file) {
const id = this._normalize(file);

return this._graph.dependenciesOf(id);
return this._graph.dependenciesOf(id, leavesOnly);
}

return this._graph.overallOrder();
return this._graph.overallOrder(leavesOnly);
}

// Get the dependant files for a file
dependents(file) {
dependents(file, options = false) {
if(!file) {
throw new Error("Must provide a file to processor.dependants()");
}

const id = this._normalize(file);
const { leavesOnly } = options;

return this._graph.dependantsOf(id);
return this._graph.dependantsOf(id, leavesOnly);
}

// Get the ultimate output for specific files or the entire tree
async output(args = false) {
const { to } = args;
let { files } = args;

if(!Array.isArray(files)) {
@@ -225,7 +234,6 @@ class Processor {

// Rewrite relative URLs before adding
// Have to do this every time because target file might be different!
//
const results = [];

for(const dep of files) {
@@ -240,7 +248,7 @@ class Processor {

params(this, {
from : dep,
to : args.to,
to,
})
);

@@ -308,6 +316,11 @@ class Processor {
return this._options;
}

// Expose the dependency graph
get graph() {
return this._graph;
}

// Return all the compositions for the files loaded into the processor instance
get compositions() {
// Ensure all files are fully-processed first
@@ -145,6 +145,12 @@ exports[`/processor.js API .invalidate() should throw if an invalid file is pass

exports[`/processor.js API .invalidate() should throw if no file is passed 1`] = `"invalidate() requires a file argument"`;

exports[`/processor.js API .normalize() should normalize inputs 1`] = `
Array [
"simple.css",
]
`;

exports[`/processor.js API .output() should allow for seperate source map output 1`] = `
Object {
"file": "to.css",
@@ -35,7 +35,6 @@ describe("/processor.js", () => {
describe(".file()", () => {
it("should process a relative file", async () => {
const result = await processor.file("./packages/processor/test/specimens/simple.css");


expect(result.exports).toMatchSnapshot();
expect(result.details.exports).toMatchSnapshot();
@@ -55,6 +54,33 @@ describe("/processor.js", () => {
});
});

describe(".has()", () => {
it("should return a boolean", async () => {
await processor.string(
"./simple.css",
".wooga { }"
);

expect(processor.has("./simple.css")).toBe(true);
expect(processor.has("./nope.css")).toBe(false);
});

it("should normalize inputs before checking for existence", async () => {
await processor.string(
"./simple.css",
".wooga { }"
);

expect(processor.has("../modular-css/simple.css")).toBe(true);
});
});

describe(".normalize()", () => {
it("should normalize inputs", async () => {
expect(relative([ processor.normalize("../modular-css/simple.css") ])).toMatchSnapshot();
});
});

describe(".remove()", () => {
it("should remove a relative file", async () => {
await processor.string(
@@ -1,5 +1,7 @@
"use strict";

const { DepGraph } = require("dependency-graph");

const namer = require("@modular-css/test-utils/namer.js");
const relative = require("@modular-css/test-utils/relative.js");
const Processor = require("../processor.js");
@@ -31,5 +33,14 @@ describe("/processor.js", () => {
expect(typeof processor.options).toBe("object")
);
});

describe(".graph", () => {
it("should return the dependency graph for added CSS files", async () => {
await processor.file("./packages/processor/test/specimens/start.css");
await processor.file("./packages/processor/test/specimens/local.css");

expect(processor.graph).toBeInstanceOf(DepGraph);
});
});
});
});
@@ -3,6 +3,7 @@
const MagicString = require("magic-string");
const dedent = require("dedent");
const escape = require("escape-string-regexp");
const { DepGraph } = require("dependency-graph");

const formats = {
es : require("./formats/es.js"),
@@ -38,20 +39,34 @@ module.exports = (opts) => {
this.error(`Unsupported format: ${format}. Supported formats are ${JSON.stringify([ ...supported.values() ])}`);
}

const entries = new Map();
const graph = new DepGraph({ circular : true });

Object.entries(chunks).forEach(([ entry, chunk ]) => {
const {
isAsset = false,
code = "",
dynamicImports = [],
} = chunk;
const { isAsset, imports, dynamicImports } = chunk;

// Guard against https://github.com/rollup/rollup/issues/2659
const deps = dynamicImports.filter(Boolean);

if(isAsset || !deps.length) {
if(isAsset) {
return;
}

// Guard against https://github.com/rollup/rollup/issues/2659
const imported = dynamicImports.filter(Boolean);

if(imported.length) {
entries.set(entry, imported);
}

graph.addNode(entry);

imported.forEach((file) => {
graph.addNode(file);
graph.addDependency(entry, file);
});
});

entries.forEach((deps, entry) => {
const { code } = chunks[entry];

const { regex, loader, load } = formats[format];

const search = regex(deps.map(escape).join("|"));
@@ -72,10 +87,17 @@ module.exports = (opts) => {
const [ statement, file ] = result;
const { index } = result;

const { dynamicAssets : assets = false } = chunks[file];
// eslint-disable-next-line no-loop-func
const css = [ ...graph.dependenciesOf(file), file ].reduce((out, curr) => {
const { assets = [] } = chunks[curr];

assets.forEach((asset) => out.add(asset));

return out;
}, new Set());

if(assets && assets.length) {
const imports = assets.map((dep) =>
if(css.size) {
const imports = [ ...css ].map((dep) =>
`${options.loadfn}("./${dep}")`
);

@@ -91,7 +113,7 @@ module.exports = (opts) => {

log("Updating", entry);

chunk.code = str.toString();
chunks[entry].code = str.toString();
});
},
};
Oops, something went wrong.

0 comments on commit e0c5eee

Please sign in to comment.
You can’t perform that action at this time.