Skip to content

Commit

Permalink
Vite updates & invalidation fixes (#972)
Browse files Browse the repository at this point in the history
- Invalidating a file invalidates all of it's selectors _AND_ values now
- When a file is re-walked all selector and value dependencies are removed, so they can be re-added by the walk with the latest information
  • Loading branch information
tivac authored Sep 26, 2023
1 parent 4fa842e commit 673df5d
Show file tree
Hide file tree
Showing 5 changed files with 115 additions and 17 deletions.
7 changes: 7 additions & 0 deletions .changeset/angry-shoes-double.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@modular-css/processor": patch
"@modular-css/vite": patch
---

Improved vite integration, now correctly invalidating files when a file is changed or deleted.
Improved invalidation in the processor, preventing stale `@value` or `composes` references from being output.
52 changes: 37 additions & 15 deletions packages/processor/processor.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ const {
filterByPrefix,

FILE_PREFIX,
SELECTOR_PREFIX,
} = keys;

let fs;
Expand Down Expand Up @@ -238,15 +237,17 @@ class Processor {
});

// Mark all selectors in the file invalid
filterByPrefix(SELECTOR_PREFIX, this._graph.dependenciesOf(key), { clean : false }).forEach((sKey) => {
const data = this._graph.getNodeData(sKey);
this._graph.dependenciesOf(key).forEach((dep) => {
const data = this._graph.getNodeData(dep);

if(data.file !== normalized) {
return;
}

data.valid = false;
});

this._log("invalidate()", normalized);
}

// Get the dependency order for a file or the entire tree
Expand All @@ -262,9 +263,23 @@ class Processor {
throw new Error(`Unknown file: ${normalized}`);
}

const dependencies = this._graph.dependenciesOf(key);
return filterByPrefix(FILE_PREFIX, this._graph.dependenciesOf(key));
}

// Get the file dependents for a specific file
fileDependents(file) {
if(!file) {
throw new Error("fileDepenendents() must be called with a file");
}

const normalized = this._normalize(file);
const key = fileKey(normalized);

if(!this._graph.hasNode(key)) {
throw new Error(`Unknown file: ${normalized}`);
}

return filterByPrefix(FILE_PREFIX, dependencies);
return filterByPrefix(FILE_PREFIX, this._graph.dependantsOf(key));
}

// Get the ultimate output for specific files or the entire tree
Expand Down Expand Up @@ -496,15 +511,6 @@ class Processor {
}

// Add selector and its dependencies to the graph
const selectorId = selectorKey(name, selector);

// Remove any existing dependencies for the selector if it is invalid
if(graph.hasNode(selectorId) && !graph.getNodeData(selectorId).valid) {
graph.dependenciesOf(selectorId).forEach((other) => {
graph.removeDependency(selectorId, other);
});
}

this._addSelector(name, selector);

refs.forEach(({ name : depSelector }) => {
Expand Down Expand Up @@ -597,7 +603,23 @@ class Processor {
return;
}

const fKey = this._addFile(name);
const fKey = fileKey(name);

// Clean up old graph dependencies for this node since it's about to be parsed
// and they'll all be recreated anyways
if(graph.hasNode(fKey)) {
graph.directDependenciesOf(fKey).forEach((dep) => {
const data = this._graph.getNodeData(dep);

if(data.file !== name) {
return;
}

graph.directDependenciesOf(dep).forEach((dep2) => graph.removeDependency(dep, dep2));
});
}

this._addFile(name);

this._log("_before()", name);

Expand Down
6 changes: 6 additions & 0 deletions packages/processor/test/__snapshots__/api.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,12 @@ exports[`/processor.js API .fileDependencies() should return the overall order o
]
`;

exports[`/processor.js API .fileDependents() should return the dependencies of the specified file 1`] = `
[
"packages/processor/test/specimens/start.css",
]
`;

exports[`/processor.js API .invalidate() should invalidate a relative file 1`] = `
[
[
Expand Down
27 changes: 27 additions & 0 deletions packages/processor/test/api.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,33 @@ describe("/processor.js", () => {
});
});

describe(".fileDependents()", () => {
it("should return the dependencies of the specified file", async () => {
await processor.file("./packages/processor/test/specimens/start.css");

expect(
relative(processor.fileDependents(require.resolve("./specimens/local.css")))
)
.toMatchSnapshot();
});

it("should throw if no file is specified", async () => {
await processor.file("./packages/processor/test/specimens/start.css");

expect(() => processor.fileDependents()).toThrow("must be called with a file");
});

it("should throw on requesting an invalid file", async () => {
await processor.string("./does/not/exist.css", dedent(`
.foo {
color: red;
}
`));

expect(() => processor.fileDependents("./also/does/not/exist.css")).toThrow("Unknown file: ");
});
});

describe(".output()", () => {
it("should reject unknown files", async () => {
await expect(processor.output({
Expand Down
40 changes: 38 additions & 2 deletions packages/vite/vite.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,26 @@ module.exports = (
}

let viteServer;
let viteConfig;

const watchFile = (file) => {
const slashed = slash(file);

if(viteServer) {
viteServer.watcher.add(slashed);
} else if(viteConfig.build.watch) {
this.addWatchFile(slashed);
}
};

return {
name : "@modular-css/vite",
enforce : "pre",

configResolved(config) {
viteConfig = config;
},

configureServer(instance) {
viteServer = instance;

Expand All @@ -75,14 +90,35 @@ module.exports = (
}

processor.invalidate(file);

// Make sure vite knows that files that depend on us might've changed too
processor.fileDependents(file).forEach((dep) => {
viteServer.watcher.emit("change", dep);
});
});

viteServer.watcher.on("unlink", (file) => {
if(!processor.has(file)) {
return;
}

// Have to grab deps _before_ removal
const deps = processor.fileDependents(file);

processor.remove(file);

// Invalidate dependencies of the file that was removed
deps.forEach((dep) => {
viteServer.watcher.emit("change", dep);
});
});
},

buildStart() {
log("build start");

// Watch any files already in the procesor
Object.keys(processor.files).forEach((file) => this.addWatchFile(file));
Object.keys(processor.files).forEach((file) => watchFile(file));
},

async resolveId(source) {
Expand Down Expand Up @@ -191,7 +227,7 @@ module.exports = (

const deps = processor.fileDependencies(file);

deps.forEach((dep) => this.addWatchFile(slash(dep)));
deps.forEach((dep) => watchFile(dep));

// TODO: Gets CSS order right by forcing them to load their dependent CSS first.
// Feels very brittle and overkill, but this is what we've got for now
Expand Down

0 comments on commit 673df5d

Please sign in to comment.