Skip to content

Commit

Permalink
feat: processor.normalize() & corrected chunking logic (#562)
Browse files Browse the repository at this point in the history
* 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 e0c5eee
Show file tree
Hide file tree
Showing 45 changed files with 1,210 additions and 553 deletions.
12 changes: 6 additions & 6 deletions packages/namer/namer.js
Expand Up @@ -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;
};
};
29 changes: 21 additions & 8 deletions packages/processor/processor.js
Expand Up @@ -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);
Expand All @@ -167,7 +172,7 @@ class Processor {
}

const deps = this.dependents(source);

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

Expand All @@ -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)) {
Expand All @@ -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) {
Expand All @@ -240,7 +248,7 @@ class Processor {

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

Expand Down Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions packages/processor/test/__snapshots__/api.test.js.snap
Expand Up @@ -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",
Expand Down
28 changes: 27 additions & 1 deletion packages/processor/test/api.test.js
Expand Up @@ -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();
Expand All @@ -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(
Expand Down
11 changes: 11 additions & 0 deletions packages/processor/test/getters.test.js
@@ -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");
Expand Down Expand Up @@ -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);
});
});
});
});
48 changes: 35 additions & 13 deletions packages/rollup-rewriter/rewriter.js
Expand Up @@ -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"),
Expand Down Expand Up @@ -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("|"));
Expand All @@ -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}")`
);

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

log("Updating", entry);

chunk.code = str.toString();
chunks[entry].code = str.toString();
});
},
};
Expand Down

0 comments on commit e0c5eee

Please sign in to comment.