Skip to content

[WIP] Ember cli classic #185

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 22 commits into from
Aug 11, 2018
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
512a11f
feat: Make ember-cli work with classic ember structure.
amiller-gh Jul 11, 2018
463fe10
feat: Finish wiring up analysis and css output for addons.
amiller-gh Jul 12, 2018
7c1c001
feat: Added functionality.
amiller-gh Jul 17, 2018
94a4a1c
fix: Make ember-cli plugin work for Glimmer apps again.
amiller-gh Jul 18, 2018
ad83f78
fix: Improve transports to not duplicate output.
amiller-gh Jul 18, 2018
85772e1
feat: Improvements to ember-cli addon.
amiller-gh Jul 23, 2018
ae2ed0a
fix: Remove last lingering async fs methods.
amiller-gh Jul 23, 2018
f669473
fix: Glimmer works again with broccoli updates.
amiller-gh Jul 23, 2018
769aa6a
chore: Document helper hack for now.
amiller-gh Jul 23, 2018
1b852a4
feat: Tweak Analyzer.analyze method to accept working directory.
amiller-gh Jul 26, 2018
f6f0150
fix: Fix existing tests.
amiller-gh Jul 26, 2018
d77b7d1
feat: Glimmer helper injection.
amiller-gh Jul 29, 2018
9712b70
chore: Depend on experimental fork of glimmer-analyzer for now.
amiller-gh Jul 29, 2018
8362757
feat: Ember Template Discovery Tests.
amiller-gh Jul 31, 2018
fe3be18
fix: Remove addons' treeForAddonStyles to work around ember-cli bug.
amiller-gh Jul 31, 2018
a78802e
fix: Build @css-blocks/glimmer to AMD so Ember can import the runtime.
amiller-gh Jul 31, 2018
58dc295
feat: Engine integration.
amiller-gh Aug 5, 2018
474862c
test: Have Travis use node versions 8 and 10.
amiller-gh Aug 5, 2018
589f283
fix: Ensure file output in broccoli-aggregator.
amiller-gh Aug 5, 2018
5566127
feat: Add global disable option for apps.
amiller-gh Aug 8, 2018
aa702ed
fix: Ensure written output file is not linked to previous tree.
amiller-gh Aug 11, 2018
af52f92
docs: Add ember-cli README.
amiller-gh Aug 11, 2018
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
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
"mock-require": "^2.0.2",
"outdent": "^0.4.1",
"perfectionist": "^2.4.0",
"postcss": "^6.0.21",
"prettier": "^1.8.2",
"remap-istanbul": "^0.9.5",
"source-map-support": "^0.4.15",
Expand Down
1 change: 0 additions & 1 deletion packages/@css-blocks/broccoli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
"debug": "^3.1.0",
"fs-extra": "^5.0.0",
"opticss": "^0.3.0",
"postcss": "^6.0.21",
"recursive-readdir": "^2.2.2",
"walk-sync": "^0.3.2"
}
Expand Down
36 changes: 24 additions & 12 deletions packages/@css-blocks/broccoli/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,30 @@
import * as fs from "fs-extra";
import * as path from "path";

import { Analyzer, BlockCompiler, StyleMapping } from "@css-blocks/core";
import { Analyzer, Block, BlockCompiler, StyleMapping } from "@css-blocks/core";
import { TemplateTypes } from "@opticss/template-api";
import * as debugGenerator from "debug";
import { OptiCSSOptions, Optimizer } from "opticss";
import * as postcss from "postcss";
import { postcss } from "opticss";
import * as readdir from "recursive-readdir";

import { BroccoliPlugin } from "./utils";

const debug = debugGenerator("css-blocks:broccoli");

export interface Transport {
id: string;
mapping: StyleMapping<keyof TemplateTypes>;
blocks: Set<Block>;
analyzer: Analyzer<keyof TemplateTypes>;
css: string;
}

export interface BroccoliOptions {
entry: string[];
output: string;
analyzer: Analyzer<keyof TemplateTypes>;
transport: {[key: string]: object};
transport: Transport;
optimization?: Partial<OptiCSSOptions>;
}

Expand All @@ -25,19 +33,21 @@ class BroccoliCSSBlocks extends BroccoliPlugin {
private analyzer: Analyzer<keyof TemplateTypes>;
private entry: string[];
private output: string;
private transport: { [key: string]: object };
private transport: Transport;
private optimizationOptions: Partial<OptiCSSOptions>;

// tslint:disable-next-line:prefer-whatever-to-any
constructor(inputNode: any, options: BroccoliOptions) {
super([inputNode], { name: "broccoli-css-blocks" });

this.entry = options.entry;
this.entry = options.entry.slice(0);
this.output = options.output;
this.analyzer = options.analyzer;
this.transport = options.transport;
this.optimizationOptions = options.optimization || {};

this.transport.css = this.transport.css ? this.transport.css : "";

if (!this.output) {
throw new Error("CSS Blocks Broccoli Plugin requires an output file name.");
}
Expand All @@ -48,11 +58,18 @@ class BroccoliCSSBlocks extends BroccoliPlugin {
let blockCompiler = new BlockCompiler(postcss, options);
let optimizer = new Optimizer(this.optimizationOptions, this.analyzer.optimizationOptions);

// When no entry points are passed, we treat *every* template as an entry point.
let discover = !this.entry.length;

// This build step is *mostly* just a pass-through of all files!
// QUESTION: Tom, is there a better way to do this in Broccoli?
let files = await readdir(this.inputPaths[0]);
for (let file of files) {
file = path.relative(this.inputPaths[0], file);

// If we're in Classic or Pods mode, every hbs file is an entry point.
if (discover && path.extname(file) === ".hbs") { this.entry.push(file); }

await fs.ensureDir(path.join(this.outputPath, path.dirname(file)));
Copy link
Member

@stefanpenner stefanpenner Jul 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let transition to sync FS functions.

  • in general they are more performant (no need to schedule to the event loop and come back), and as we are processing serially through a build. Rarely (if every) does the non-blocking aspect help.
  • they make lots of sense for a webserver, as that way we don't accidentally block 1 request, do to a second request doing sync IO on a slow FS.
  • typically sync errors have better stack traces (although maybe newer node's fixed this)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this foreclose on the ability to run multiple Broccoli plugins concurrently in the future?

Copy link
Contributor Author

@amiller-gh amiller-gh Aug 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe so. We're already running multiple instances of this broccoli plugin (one for each addon, engine, or app). Broccoli (according to spenner) runs the build step synchronously already, so we don't actually gain any perf benefits from async functions and instead are just left with that extra throwaway promise being created for each file system call.

If Broccoli begins running certain build steps asynchronously, this may be a perf bottleneck, but we'll need to update synchronous fs calls across the entire ecosystem to un-block it.

From a CSS Blocks perspective, there is no difference in sync vs async.

try {
await fs.symlink(
Expand Down Expand Up @@ -80,7 +97,6 @@ class BroccoliCSSBlocks extends BroccoliPlugin {
let filename = filesystemPath || options.importer.debugIdentifier(block.identifier, options);

// If this Block has a representation on disk, remove it from our output tree.
// TODO: This isn't working right now because `importer.filesystemPath` doesn't return the expected path...
if (filesystemPath) {
debug(`Removing block file ${path.relative(options.rootDir, filesystemPath)} from output.`);
await fs.unlink(path.join(this.outputPath, path.relative(options.rootDir, filesystemPath)));
Expand All @@ -106,13 +122,9 @@ class BroccoliCSSBlocks extends BroccoliPlugin {
this.transport.mapping = styleMapping;
this.transport.blocks = blocks;
this.transport.analyzer = this.analyzer;
this.transport.css = optimized.output;
this.transport.css += optimized.output.content.toString();

// Write our compiled CSS to the output tree.
await fs.writeFile(
path.join(this.outputPath, this.output),
optimized.output.content.toString(),
);
debug(`Compilation Finished: ${this.transport.id}`);

}

Expand Down
2 changes: 1 addition & 1 deletion packages/@css-blocks/broccoli/test/plugin-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ describe("Broccoli Plugin Test", function () {
});

let transport = {};
let analyzer = new GlimmerAnalyzer(input.path(), {
let analyzer = new GlimmerAnalyzer(input.path(), ".", {
app: { name: "test" },
types: {
stylesheet: { definitiveCollection: "components" },
Expand Down
10 changes: 10 additions & 0 deletions packages/@css-blocks/ember-cli/addon/helpers/classnames.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { helper } from "@ember/component/helper";

// For some reason Ember doesn't include the helper runtime from `@css-blocks/glimmer`
// in the build and throws a "missing module" error at runtime. The contents have been
// temporarily copied to `./tmp` as a workaround instead... this will block release.
// import { classnames } from "@css-blocks/glimmer/dist/src/helpers/classnames";
import { classnames } from "./tmp";

export default helper(classnames);
export { classnames };
5 changes: 5 additions & 0 deletions packages/@css-blocks/ember-cli/addon/helpers/concat.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { concat } from "@css-blocks/glimmer/dist/src/helpers/concat";
import { helper } from "@ember/component/helper";

export default helper(concat);
export { concat };
124 changes: 124 additions & 0 deletions packages/@css-blocks/ember-cli/addon/helpers/tmp.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@

const e = (m) => { throw new Error(m); };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this module looks pretty complex, do we have associated unit tests?

Copy link
Contributor Author

@amiller-gh amiller-gh Jul 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy pasta from the unit tested version in @css-blocks/glimmer because Ember isn't resolving it at build time – issue is commented in the helpers file. This will have to be resolved before it can be merged.

const toStr = (s) => typeof s === "symbol" ? s.toString() : "" + s;
const num = (v) => typeof v[0] === "number" ? v.shift() : e("not a number: " + toStr(v[0]));
const str = (s) => toStr(s.shift());
const truthyString = (v) => {
let s = v.shift();
if (!s && s !== 0)
return;
return s.toString();
};
const bool = (v) => !!v.shift();
function classnames(stack) {
stack = stack.slice(0);
let sources = [];
let classes = [];
let nSources = num(stack);
let nOutputs = num(stack);
let canSetSource = true;
let abort = () => canSetSource = false;
let isSourceSet = (n) => sources[n];
let setSource = (n) => { if (canSetSource)
sources[n] = true; };
while (nSources-- > 0) {
sourceExpr(stack, isSourceSet, setSource, abort);
canSetSource = true;
}
while (nOutputs-- > 0) {
let c = str(stack);
if (boolExpr(stack, isSourceSet))
classes.push(c);
}
return classes.join(" ");
}

function sourceExpr(stack, isSourceSet, setSource, abort) {
let enforceSwitch = true;
let type = num(stack);
if (type & 1 /* dependency */) {
let numDeps = num(stack);
while (numDeps-- > 0) {
let depIndex = num(stack);
if (!isSourceSet(depIndex))
enforceSwitch = abort();
}
}
if (type & 2 /* boolean */) {
if (!bool(stack))
abort();
}
if (type & 4 /* switch */) {
let nValues = num(stack);
let ifFalsy = num(stack);
let value = truthyString(stack);
if (value === undefined) {
switch (ifFalsy) {
case 2 /* default */:
value = str(stack);
break;
case 0 /* error */:
if (enforceSwitch)
e("string expected"); // TODO: error message
break;
case 1 /* unset */:
abort();
break;
default:
e("wtf");
}
}
while (nValues-- > 0) {
let matchValue = str(stack);
let nSources = num(stack);
while (nSources-- > 0) {
value === matchValue ? setSource(num(stack)) : num(stack);
}
}
}
else if (type === 0 /* ternary */) {
let condition = bool(stack);
let nTrue = num(stack);
while (nTrue-- > 0) {
condition ? setSource(num(stack)) : num(stack);
}
let nFalse = num(stack);
while (nFalse-- > 0) {
condition ? num(stack) : setSource(num(stack));
}
}
else {
let nSources = num(stack);
while (nSources-- > 0) {
setSource(num(stack));
}
}
}
function boolExpr(stack, isSourceSet) {
let result;
let type = num(stack);
switch (type) {
case -1 /* not */:
return !boolExpr(stack, isSourceSet);
case -3 /* and */:
let nAnds = num(stack);
result = true;
while (nAnds-- > 0) {
let nextResult = boolExpr(stack, isSourceSet);
result = result && nextResult;
}
return result;
case -2 /* or */:
let nOrs = num(stack);
result = false;
while (nOrs-- > 0) {
let nextResult = boolExpr(stack, isSourceSet);
result = result || nextResult;
}
return result;
default:
return isSourceSet(type);
}
}

export { classnames };
1 change: 1 addition & 0 deletions packages/@css-blocks/ember-cli/app/helpers/classnames.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default, classnames } from '@css-blocks/ember-cli/helpers/classnames';
1 change: 1 addition & 0 deletions packages/@css-blocks/ember-cli/app/helpers/concat.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default, concat } from '@css-blocks/ember-cli/helpers/concat';
Loading