Skip to content

fix(broccoli): Rebuild diffs input changes and output patch separately. #197

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

Merged
merged 7 commits into from
Sep 13, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ language: node_js

node_js:
- "8"
- "10"
- "10.4.1"

env:
- CXX=g++-4.8
Expand All @@ -15,6 +15,7 @@ addons:
chrome: stable

before_install:
<<<<<<< HEAD
- npm install -g lerna yarn
- ./scripts/if-opticss-dev.sh ./scripts/checkout-opticss.sh ../opticss
- ./scripts/if-opticss-dev.sh ./scripts/link-to-opticss.js --file ../opticss
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
"markdown-toc": "^1.2.0",
"mocha": "^3.4.2",
"mocha-typescript": "^1.0.23",
"mock-fs": "^4.3.0",
"mock-fs": "4.6.0",
"mock-require": "^2.0.2",
"outdent": "^0.4.1",
"perfectionist": "^2.4.0",
Expand Down
3 changes: 1 addition & 2 deletions packages/@css-blocks/broccoli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
"@css-blocks/code-style": "^0.18.0",
"@css-blocks/glimmer": "^0.20.0-beta.0",
"@types/glob": "^5.0.35",
"broccoli-test-helper": "^1.4.0",
"broccoli-test-helper": "^1.5.0",
"watch": "^1.0.2"
},
"dependencies": {
Expand All @@ -41,7 +41,6 @@
"broccoli-funnel": "^2.0.1",
"broccoli-merge-trees": "^3.0.0",
"broccoli-plugin": "^1.3.0",
"broccoli-test-helper": "^1.2.0",
"colors": "^1.2.1",
"debug": "^3.1.0",
"fs-extra": "^5.0.0",
Expand Down
26 changes: 13 additions & 13 deletions packages/@css-blocks/broccoli/src/Analyze.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ export class CSSBlocksAnalyze extends BroccoliPlugin {
private root: string;
private transport: Transport;
private optimizationOptions: Partial<OptiCSSOptions>;
private previous: FSTree = new FSTree();
private previousInput: FSTree = new FSTree();
private previousOutput: FSTree = new FSTree();

/**
* Initialize this new instance with the app tree, transport, and analysis options.
Expand Down Expand Up @@ -72,29 +73,29 @@ export class CSSBlocksAnalyze extends BroccoliPlugin {

// Test if anything has changed since last time. If not, skip all analysis work.
let newFsTree = FSTree.fromEntries(walkSync.entries(input));
let diff = this.previous.calculatePatch(newFsTree);
let diff = this.previousInput.calculatePatch(newFsTree);
if (!diff.length) { return; }
this.previous = newFsTree;
FSTree.applyPatch(input, output, diff);
FSTree.applyPatch(input, output, this.previousOutput.calculatePatch(newFsTree));
this.previousInput = newFsTree;

// When no entry points are passed, we treat *every* template as an entry point.
this.entries = this.entries.length ? this.entries : glob.sync("**/*.hbs", { cwd: input });
this.entries = this.entries.length ? this.entries : glob.sync("**/*.hbs", { cwd: output });

// The glimmer-analyzer package tries to require() package.json
// in the root of the directory it is passed. We pass it our broccoli
// tree, so it needs to contain package.json too.
// TODO: Ideally this is configurable in glimmer-analyzer. We can
// contribute that option back to the project. However,
// other template integrations may want this available too...
let pjsonLink = path.join(input, "package.json");
let pjsonLink = path.join(output, "package.json");
if (!fs.existsSync(pjsonLink)) {
symlinkOrCopy(path.join(this.root, "package.json"), pjsonLink);
}

// Oh hey look, we're analyzing.
this.analyzer.reset();
this.transport.reset();
await this.analyzer.analyze(input, this.entries);
await this.analyzer.analyze(output, this.entries);

// Compile all Blocks and add them as sources to the Optimizer.
// TODO: handle a sourcemap from compiling the block file via a preprocessor.
Expand All @@ -108,9 +109,8 @@ export class CSSBlocksAnalyze extends BroccoliPlugin {

// If this Block has a representation on disk, remove it from our output tree.
if (filesystemPath) {
let outputStylesheet = path.join(output, path.relative(input, filesystemPath));
debug(`Removing block file ${outputStylesheet} from output.`);
if (fs.existsSync(outputStylesheet)) { fs.removeSync(outputStylesheet); }
debug(`Removing block file ${filesystemPath} from output.`);
if (fs.existsSync(filesystemPath)) { fs.removeSync(filesystemPath); }
}

// Add the compiled Block file to the optimizer.
Expand All @@ -122,6 +122,9 @@ export class CSSBlocksAnalyze extends BroccoliPlugin {
}
}

// Save the current state of our output dir for future diffs.
this.previousOutput = FSTree.fromEntries(walkSync.entries(output));

// Add each Analysis to the Optimizer.
this.analyzer.eachAnalysis((a) => optimizer.addAnalysis(a.forOptimizer(options)));

Expand All @@ -134,9 +137,6 @@ export class CSSBlocksAnalyze extends BroccoliPlugin {
this.transport.blocks = blocks;
this.transport.analyzer = this.analyzer;
this.transport.css += optimized.output.content.toString();

debug(`Compilation Finished: ${this.transport.id}`);

}

}
5 changes: 5 additions & 0 deletions packages/@css-blocks/broccoli/test/Aggregate.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import * as assert from "assert";

import { TempDir, createBuilder, createTempDir } from "broccoli-test-helper";
import * as FSTree from "fs-tree-diff";
import * as walkSync from "walk-sync";

import { CSSBlocksAggregate, Transport } from "../src/index";

Expand Down Expand Up @@ -41,8 +43,11 @@ describe("Broccoli Aggregate Plugin Test", function () {
let output = createBuilder(pluginC);

// First pass does full compile and copies all files except block files to output.
let preDiff = FSTree.fromEntries(walkSync.entries(input.path()));
await output.build();
let postDiff = FSTree.fromEntries(walkSync.entries(input.path()));

assert.equal(preDiff.calculatePatch(postDiff).length, 0, "Input directory unchanged after build.");
assert.deepEqual(output.changes(), {
"test.css": "create",
"app.css": "create",
Expand Down
8 changes: 8 additions & 0 deletions packages/@css-blocks/broccoli/test/Analyze.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import * as assert from "assert";

import { GlimmerAnalyzer } from "@css-blocks/glimmer";

import { TempDir, createBuilder, createTempDir } from "broccoli-test-helper";
import * as FSTree from "fs-tree-diff";
import * as walkSync from "walk-sync";

import { CSSBlocksAnalyze, Transport } from "../src/index";

Expand Down Expand Up @@ -57,7 +60,11 @@ describe("Broccoli Analyze Plugin Test", function () {
));

// First pass does full compile and copies all files except block files to output.
let preDiff = FSTree.fromEntries(walkSync.entries(input.path()));
await output.build();
let postDiff = FSTree.fromEntries(walkSync.entries(input.path()));

assert.equal(preDiff.calculatePatch(postDiff).length, 0, "Input directory unchanged after build.");
assert.ok(Object.keys(transport).length, "Transport Object populated");
assert.ok(transport["mapping"], "Mapping property is populated in Transport Object");
assert.ok(transport["blocks"], "Blocks property is populated in Transport Object");
Expand All @@ -79,6 +86,7 @@ describe("Broccoli Analyze Plugin Test", function () {
}}}},
});
await output.build();
assert.equal(preDiff.calculatePatch(postDiff).length, 0, "Input directory unchanged after rebuild.");
assert.equal(transport["css"], ".a { color: blue; } .b { color: yellow; }", "Modifications to block files trigger build but result in no output tree changes.");
assert.deepEqual(output.changes(), {}, "Modifications to block files trigger build but result in no output tree changes.");

Expand Down
4 changes: 2 additions & 2 deletions packages/@css-blocks/ember-cli/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -227,10 +227,10 @@ module.exports = {


if (options.output !== undefined && typeof options.output !== "string") {
throw new Error("Invalid css-block options in `ember-cli-build.js`: Output must be a string or array.");
throw new Error(`Invalid css-blocks options in 'ember-cli-build.js': Output must be a string or array. Instead received ${options.output}.`);
}
if (!isEmber && typeof options.entry !== "string" && !Array.isArray(options.entry)) {
throw new Error("Invalid css-block options in `ember-cli-build.js`: Entry must be a string or array.");
throw new Error(`Invalid css-blocks options in 'ember-cli-build.js': Entry must be a string or array. Instead received ${options.entry}.`);
}
if (isEmber && options.entry) {
throw new Error(`CSS Blocks entry points are auto-discovered in Ember apps. Do not pass an "entry" option in your CSS Blocks config.`);
Expand Down

This file was deleted.

1 change: 0 additions & 1 deletion packages/@css-blocks/ember-cli/tests/dummy/app/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ Router.map(function() {
this.route('addon-component');
this.mount('in-repo-engine');
this.mount('in-repo-lazy-engine');
this.route('node-modules-resolution');
});

export default Router;

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@
<li>
{{#link-to "in-repo-lazy-engine" class="test-nav-item"}}In Repo Lazy Engine{{/link-to}}
</li>
<li>
{{#link-to "node-modules-resolution" class="test-nav-item"}}Node Module Resolution{{/link-to}}
</li>
</ul>
</nav>

Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"ember-addon"
],
"dependencies": {
"@css-blocks/ember-cli": "*",
"@css-blocks/ember-cli": "^0.20.0-beta.0",
"ember-cli": "*",
"ember-cli-htmlbars": "*",
"ember-cli-babel": "*"
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"ember-engine"
],
"dependencies": {
"@css-blocks/ember-cli": "*",
"@css-blocks/ember-cli": "^0.20.0-beta.0",
"ember-cli": "*",
"ember-cli-htmlbars": "*",
"ember-cli-babel": "*",
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"ember-engine"
],
"dependencies": {
"@css-blocks/ember-cli": "*",
"@css-blocks/ember-cli": "^0.20.0-beta.0",
"ember-cli": "*",
"ember-cli-htmlbars": "*",
"ember-cli-babel": "*"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import Analyzer from "@amiller-gh/glimmer-analyzer";

import { expect } from "chai";
import Analyzer from "glimmer-analyzer";

import { fixture } from "./fixtures";

Expand All @@ -26,14 +27,14 @@ describe("Recursive template dependency analysis", function() {
let map = analyzer.resolutionMapForEntryPoint("my-app");

expect(map).to.deep.equal({
"component:/basic-app/components/my-app": "src/ui/components/my-app/component.ts",
"template:/basic-app/components/my-app": "src/ui/components/my-app/template.hbs",
"component:/basic-app/components/my-app/page-banner": "src/ui/components/my-app/page-banner/component.ts",
"template:/basic-app/components/my-app/page-banner": "src/ui/components/my-app/page-banner/template.hbs",
"template:/basic-app/components/ferret-launcher": "src/ui/components/ferret-launcher/template.hbs",
"template:/basic-app/components/my-app/page-banner/user-avatar": "src/ui/components/my-app/page-banner/user-avatar/template.hbs",
"template:/basic-app/components/text-editor": "src/ui/components/text-editor/template.hbs",
"component:/basic-app/components/text-editor": "src/ui/components/text-editor/component.ts",
"component:/basic-app/components/my-app": "ui/components/my-app/component.ts",
"template:/basic-app/components/my-app": "ui/components/my-app/template.hbs",
"component:/basic-app/components/my-app/page-banner": "ui/components/my-app/page-banner/component.ts",
"template:/basic-app/components/my-app/page-banner": "ui/components/my-app/page-banner/template.hbs",
"template:/basic-app/components/ferret-launcher": "ui/components/ferret-launcher/template.hbs",
"template:/basic-app/components/my-app/page-banner/user-avatar": "ui/components/my-app/page-banner/user-avatar/template.hbs",
"template:/basic-app/components/text-editor": "ui/components/text-editor/template.hbs",
"component:/basic-app/components/text-editor": "ui/components/text-editor/component.ts",
});
});
});
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import Analyzer from "@amiller-gh/glimmer-analyzer";

import { expect } from "chai";
import Analyzer from "glimmer-analyzer";

import { fixture } from "./fixtures";

Expand Down
19 changes: 9 additions & 10 deletions packages/@css-blocks/jsx/src/Analyzer/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { some, unwrap } from "@opticss/util";
import traverse from "babel-traverse";
import * as babylon from "babylon";
import * as debugGenerator from "debug";
import * as fs from "fs";
import * as fs from "fs-extra";
import * as path from "path";

import { CssBlocksJSXOptions } from "../options";
Expand Down Expand Up @@ -143,15 +143,14 @@ export class CSSBlocksJSXAnalyzer extends Analyzer<TEMPLATE_TYPE> {
* @param file The file path to read in and parse.
* @param opts Optional analytics parser options.
*/
public parseFile(file: string): Promise<JSXAnalysis> {
public async parseFile(file: string): Promise<JSXAnalysis> {
let data;
file = path.resolve(this.options.baseDir, file);
return new Promise((resolve, reject) => {
fs.readFile(file, "utf8", (err, data) => {
if (err) {
reject(new JSXParseError(`Cannot read JSX entry point file ${file}: ${err.message}`, { filename: file }));
}
resolve(this.parse(file, data));
});
});
try {
data = await fs.readFile(file, "utf8");
} catch (err) {
throw new JSXParseError(`Cannot read JSX entry point file ${file}: ${err.message}`, { filename: file });
}
return this.parse(file, data);
}
}
Loading