Skip to content
This repository has been archived by the owner on Dec 7, 2021. It is now read-only.

Commit

Permalink
Better handling of file paths vs URLs
Browse files Browse the repository at this point in the history
  • Loading branch information
@justinfagnani
justinfagnani committed May 26, 2016
1 parent 790d85a commit ce25cf0
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 36 deletions.
49 changes: 33 additions & 16 deletions src/build/analyzer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,15 @@ import * as fs from 'fs';
import {Analyzer, Deferred, Loader, Resolver, DocumentDescriptor} from 'hydrolysis';
import {posix as posixPath} from 'path';
import * as path from 'path';
import * as osPath from 'path';
import {Transform} from 'stream';
import File = require('vinyl');
import {parse as parseUrl} from 'url';
import * as logging from 'plylog';
import {Node, queryAll, predicates, getAttribute} from 'dom5';

const minimatchAll = require('minimatch-all');
const logger = logging.getLogger('cli.build.analyzer');

export interface DocumentDeps {
imports?: Array<string>;
Expand Down Expand Up @@ -93,8 +95,12 @@ export class StreamAnalyzer extends Transform {
_flush(done: (error?) => void) {
this._getDepsToEntrypointIndex().then((depsIndex) => {
// push held back files
for (let entrypoint of this.allFragments) {
let file = this.files.get(entrypoint);
for (let fragment of this.allFragments) {
let url = this.urlFromPath(fragment)
let file = this.files.get(url);
if (file == null) {
done(new Error(`no file found for fragment ${fragment}`));
}
this.push(file);
}
this._analyzeResolve(depsIndex);
Expand All @@ -108,23 +114,30 @@ export class StreamAnalyzer extends Transform {
* shared-bundle.html. This should probably be refactored so that the files
* can be injected into the stream.
*/
addFile(file) {
addFile(file: File) {
// Store only root-relative paths, in URL/posix format
if (!file.path.startsWith(this.root)) {
throw new Error(`Received a file not in root: ${file.path} (${this.root})`);
}
let localPath = posixPath.normalize(path.relative(this.root, file.path));
this.files.set(localPath, file);
this.files.set(this.urlFromPath(file.path), file);
}

getFile(filepath: string) {
return this.files.get(this.urlFromPath(filepath));
}

isFragment(file): boolean {
return this.allFragments.indexOf(file.path) !== -1;
}

urlFromPath(filepath) {
if (!filepath.startsWith(this.root)) {
throw new Error(`file path is not in root: ${filepath} (${this.root})`);
}
// convert filesystem path to URL
return path.normalize(osPath.relative(this.root, filepath));
}

_getDepsToEntrypointIndex(): Promise<DepsIndex> {
// TODO: tsc is being really weird here...
let depsPromises = <Promise<DocumentDeps>[]>this.allFragments.map(
(e) => this._getDependencies(e));
let depsPromises = <Promise<DepsIndex>[]>this.allFragments.map((f) =>
this._getDependencies(this.urlFromPath(f)));

return Promise.all(depsPromises).then((value: any) => {
// tsc was giving a spurious error with `allDeps` as the parameter
Expand Down Expand Up @@ -170,13 +183,13 @@ export class StreamAnalyzer extends Transform {
* Attempts to retreive document-order transitive dependencies for `url`.
*/
_getDependencies(url: string): Promise<DocumentDeps> {
let documents = this.analyzer.parsedDocuments;
let dir = path.dirname(url);
return this.analyzer.metadataTree(url)
.then((tree) => this._getDependenciesFromDescriptor(tree, dir));
}

_getDependenciesFromDescriptor(descriptor: DocumentDescriptor, dir: string): DocumentDeps {
_getDependenciesFromDescriptor(descriptor: DocumentDescriptor, dir: string)
: DocumentDeps {
let allHtmlDeps = [];
let allScriptDeps = new Set<string>();
let allStyleDeps = new Set<string>();
Expand Down Expand Up @@ -250,13 +263,17 @@ class StreamResolver implements Resolver {
return false;
}

let filepath = decodeURIComponent(urlObject.pathname);
let file = this.analyzer.files.get(filepath);
let urlPath = decodeURIComponent(urlObject.pathname);
let file = this.analyzer.files.get(urlPath);

if (file) {
deferred.resolve(file.contents.toString());
} else {
throw new Error(`No file found for ${filepath}`);
logger.debug(`No file found for ${urlPath}`);
// If you're template to do the next line, Loader does that for us, so
// don't double reject!
// deferred.reject(new Error(`No file found for ${urlPath}`));
return false;
}
return true;
}
Expand Down
37 changes: 26 additions & 11 deletions src/build/bundle.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ export class Bundler extends Transform {
_flush(done: (error?) => void) {
this._buildBundles().then((bundles: Map<string, string>) => {
for (let fragment of this.allFragments) {
let file = this.analyzer.files.get(fragment);
let file = this.analyzer.getFile(fragment);
console.assert(file != null);
let contents = bundles.get(fragment);
file.contents = new Buffer(contents);
this.push(file);
Expand All @@ -103,23 +104,34 @@ export class Bundler extends Transform {
return this.allFragments.indexOf(file.path) !== -1;
}

urlFromPath(filepath) {
if (!filepath.startsWith(this.root)) {
throw new Error(`file path is not in root: ${filepath} (${this.root})`);
}
// convert filesystem path to URL
return path.normalize(osPath.relative(this.root, filepath));
}

_buildBundles(): Promise<Map<string, string>> {
return this._getBundles().then((bundles) => {
let sharedDepsBundle = this.shell || this.sharedBundleUrl;
let sharedDepsBundle = (this.shell)
? this.urlFromPath(this.shell)
: this.sharedBundleUrl;
let sharedDeps = bundles.get(sharedDepsBundle) || [];
let promises = [];

if (this.shell) {
let shellFile = this.analyzer.files.get(this.shell);
let shellFile = this.analyzer.getFile(this.shell);
console.assert(shellFile != null);
let newShellContent = this._addSharedImportsToShell(bundles);
shellFile.contents = new Buffer(newShellContent);
}

for (let fragment of this.allFragments) {
let fragmentUrl = this.urlFromPath(fragment);
let addedImports = (fragment === this.shell && this.shell)
? []
: [path.relative(path.dirname(fragment), sharedDepsBundle)];
: [path.relative(path.dirname(fragmentUrl), sharedDepsBundle)];
let excludes = (fragment === this.shell && this.shell)
? []
: sharedDeps.concat(sharedDepsBundle);
Expand All @@ -132,7 +144,7 @@ export class Bundler extends Transform {
stripExcludes: excludes,
inlineScripts: true,
inlineCss: true,
inputUrl: fragment,
inputUrl: fragmentUrl,
});
vulcanize.process(null, (err, doc) => {
if (err) {
Expand Down Expand Up @@ -164,10 +176,11 @@ export class Bundler extends Transform {

_addSharedImportsToShell(bundles: Map<string, string[]>): string {
console.assert(this.shell != null);
let shellDeps = bundles.get(this.shell)
.map((d) => path.relative(path.dirname(this.shell), d));
let shellUrl = this.urlFromPath(this.shell);
let shellDeps = bundles.get(shellUrl)
.map((d) => path.relative(path.dirname(shellUrl), d));

let file = this.analyzer.files.get(this.shell);
let file = this.analyzer.getFile(this.shell);
console.assert(file != null);
let contents = file.contents.toString();
let doc = dom5.parse(contents);
Expand Down Expand Up @@ -264,19 +277,21 @@ export class Bundler extends Transform {
// conflicting orders between their top level imports. The shell should
// always come first.
for (let fragment of fragmentToDeps.keys()) {

let fragmentUrl = this.urlFromPath(fragment);
let dependencies = fragmentToDeps.get(fragment);
for (let dep of dependencies) {
let fragmentCount = depsToEntrypoints.get(dep).length;
if (fragmentCount > 1) {
if (this.shell) {
addImport(this.shell, dep);
addImport(this.urlFromPath(this.shell), dep);
// addImport(entrypoint, this.shell);
} else {
addImport(this.sharedBundleUrl, dep);
addImport(fragment, this.sharedBundleUrl);
addImport(fragmentUrl, this.sharedBundleUrl);
}
} else {
addImport(fragment, dep);
addImport(fragmentUrl, dep);
}
}
}
Expand Down
11 changes: 7 additions & 4 deletions test/build/analyzer_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ const path = require('path');
const StreamAnalyzer = require('../../lib/build/analyzer').StreamAnalyzer;
const vfs = require('vinyl-fs-fake');

suite('Analzyer', () => {
suite('Analyzer', () => {

suite('DepsIndex', () => {

test('fragment to deps list has only uniques', (done) => {
let root = path.resolve('test/build/analyzer');
let analyzer = new StreamAnalyzer(root, null, null, [
'a.html',
'b.html',
path.resolve(root, 'a.html'),
path.resolve(root, 'b.html'),
]);
vfs.src(path.join(root, '**'), {cwdbase: true})
.pipe(analyzer)
Expand All @@ -40,7 +40,10 @@ suite('Analzyer', () => {

test("analyzing shell and entrypoint doesn't double load files", (done) => {
let root = path.resolve('test/build/analyzer');
let analyzer = new StreamAnalyzer(root, 'entrypoint.html', 'shell.html');
let analyzer = new StreamAnalyzer(
root,
path.resolve(root, 'entrypoint.html'),
path.resolve(root, 'shell.html'));
vfs.src(root + '/**', {cwdbase: true})
.pipe(analyzer)
.on('finish', () => {
Expand Down
14 changes: 10 additions & 4 deletions test/build/bundle_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,15 @@ suite('Bundler', () => {
let files;

let setupTest = (options) => new Promise((resolve, reject) => {
let fragments = options.fragments || [];
let analyzer = new StreamAnalyzer(root, options.entrypoint, options.shell, fragments);
bundler = new Bundler(root, options.entrypoint, options.shell, fragments, analyzer);
let fragments = options.fragments
? options.fragments.map((f) => path.resolve(root, f))
: [];
let entrypoint = options.entrypoint
&& path.resolve(root, options.entrypoint);
let shell = options.shell
&& path.resolve(root, options.shell);
let analyzer = new StreamAnalyzer(root, entrypoint, shell, fragments);
bundler = new Bundler(root, entrypoint, shell, fragments, analyzer);
sourceStream = new stream.Readable({
objectMode: true,
});
Expand Down Expand Up @@ -97,7 +103,7 @@ suite('Bundler', () => {
}).then((files) => {
let doc = dom5.parse(getFile('entrypointA.html'));
assert.isTrue(hasMarker(doc, 'framework'));
assert.isFalse(hasImport(doc, '/root/framework.html'));
assert.isFalse(hasImport(doc, 'framework.html'));
// TODO(justinfagnani): check that shared-bundle.html doesn't exist
// it's in the analyzer's file map for some reason
}));
Expand Down
1 change: 0 additions & 1 deletion tsconfig.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
"src/build/html-project.ts",
"src/build/optimize.ts",
"src/build/prefetch.ts",
"src/build/stream-resolver.ts",
"src/build/streams.ts",
"src/build/sw-precache.ts",
"src/build/uglify-transform.ts",
Expand Down

0 comments on commit ce25cf0

Please sign in to comment.