Skip to content

Commit

Permalink
Infer bundle type (app vs test) by original tree if known.
Browse files Browse the repository at this point in the history
Prior to these changes, we would decide what bundle a given file should
end up in based on the modules relativePath that imported it. This
worked generally for application level tests that import external
packages, as well as for many addon-test-support imports.

Unfortunately, this relative path analysis falls down in addons that
intentionally customize their `treeForAddonTestSupport` so that the
`addon-test-support/` folder contents are available **without** the
`addon-name/test-support` suffix. This is done by a number of testing
only addons, but the largest offenders are `ember-qunit` and
`@ember/test-helpers`.

This introduces a new (possibly undefined) value that can be passed into
`preprocessJs` to specify the `treeType` being transpiled. ember-cli
will update to pass this value by default, but any addons doing the sort
of overrides mentioned above are _likely_ already doing a manual
override and can easily pass in the treeType in their overrides.
  • Loading branch information
rwjblue committed Dec 10, 2020
1 parent b558c7c commit 07ecb9a
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 23 deletions.
19 changes: 13 additions & 6 deletions packages/ember-auto-import/ts/analyzer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import FSTree from 'fs-tree-diff';
import makeDebug from 'debug';
import { join, extname } from 'path';
import { isEqual, flatten } from 'lodash';
import Package from './package';
import type Package from './package';
import symlinkOrCopy from 'symlink-or-copy';
import { TransformOptions } from '@babel/core';
import { File } from '@babel/types';
Expand All @@ -18,7 +18,8 @@ makeDebug.formatters.m = (modules: Import[]) => {
specifier: m.specifier,
path: m.path,
isDynamic: m.isDynamic,
package: m.package.name
package: m.package.name,
treeType: m.treeType
})),
null,
2
Expand All @@ -27,11 +28,14 @@ makeDebug.formatters.m = (modules: Import[]) => {

const debug = makeDebug('ember-auto-import:analyzer');

export type TreeType = 'app' | 'addon' | 'addon-test-support' | 'test';

export interface Import {
path: string;
package: Package;
specifier: string;
isDynamic: boolean;
treeType: TreeType | undefined;
}

/*
Expand All @@ -45,7 +49,7 @@ export default class Analyzer extends Plugin {

private parse: undefined | ((source: string) => File);

constructor(inputTree: Node, private pack: Package) {
constructor(inputTree: Node, private pack: Package, private treeType?: TreeType) {
super([inputTree], {
annotation: 'ember-auto-import-analyzer',
persistentOutput: true
Expand Down Expand Up @@ -172,7 +176,8 @@ export default class Analyzer extends Plugin {
isDynamic: true,
specifier: argument.value,
path: relativePath,
package: this.pack
package: this.pack,
treeType: this.treeType,
});
}
},
Expand All @@ -181,7 +186,8 @@ export default class Analyzer extends Plugin {
isDynamic: false,
specifier: path.node.source.value,
path: relativePath,
package: this.pack
package: this.pack,
treeType: this.treeType,
});
},
ExportNamedDeclaration: (path) => {
Expand All @@ -190,7 +196,8 @@ export default class Analyzer extends Plugin {
isDynamic: false,
specifier: path.node.source.value,
path: relativePath,
package: this.pack
package: this.pack,
treeType: this.treeType,
});
}
}
Expand Down
8 changes: 5 additions & 3 deletions packages/ember-auto-import/ts/auto-import.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import Splitter from './splitter';
import Bundler from './bundler';
import Analyzer from './analyzer';
import type { TreeType } from './analyzer';
import Package from './package';
import { buildDebugCallback } from 'broccoli-debug';
import BundleConfig from './bundle-config';
Expand All @@ -16,7 +17,7 @@ const debugTree = buildDebugCallback('ember-auto-import');
// what you're doing.
export interface AutoImportSharedAPI {
isPrimary(addonInstance: AddonInstance): boolean;
analyze(tree: Node, addon: AddonInstance): Node;
analyze(tree: Node, addon: AddonInstance, treeType?: TreeType): Node;
included(addonInstance: AddonInstance): void;
updateFastBootManifest(manifest: { vendorFiles: string[] }): void;
}
Expand Down Expand Up @@ -57,12 +58,13 @@ export default class AutoImport implements AutoImportSharedAPI {
return this.primaryPackage === addon;
}

analyze(tree: Node, addon: AddonInstance) {
analyze(tree: Node, addon: AddonInstance, treeType?: TreeType) {
let pack = Package.lookupParentOf(addon);
this.packages.add(pack);
let analyzer = new Analyzer(
debugTree(tree, `preprocessor:input-${this.analyzers.size}`),
pack
pack,
treeType
);
this.analyzers.set(analyzer, pack);
return analyzer;
Expand Down
21 changes: 21 additions & 0 deletions packages/ember-auto-import/ts/bundle-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ import { dirname } from 'path';
import { AppInstance } from './ember-cli-models';
const testsPattern = new RegExp(`^/?[^/]+/(tests|test-support)/`);

import type { TreeType } from './analyzer';

function exhausted(value: never): never {
throw new Error(`Unknown tree type specified: ${value}`);
}

export default class BundleConfig {
constructor(private emberApp: AppInstance) {}

Expand Down Expand Up @@ -40,6 +46,21 @@ export default class BundleConfig {
}
}

bundleForTreeType(treeType: TreeType) {
switch (treeType) {
case 'app':
case 'addon':
return 'app';

case 'addon-test-support':
case 'test':
return 'test';

default:
exhausted(treeType);
}
}

// For any relative path to a module in our application, return which bundle its
// imports go into.
bundleForPath(path: string): string {
Expand Down
11 changes: 9 additions & 2 deletions packages/ember-auto-import/ts/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,15 @@ module.exports = {
// it will see all the consumer app or addon's javascript
registry.add('js', {
name: 'ember-auto-import-analyzer',
toTree: (tree: Node) => {
return AutoImport.lookup(this).analyze(tree, this);
toTree: (tree: Node, _inputPath: string, _outputPath: string, options: any) => {

let treeType;

if (typeof options === 'object' && options !== null && options.treeType) {
treeType = options.treeType;
}

return AutoImport.lookup(this).analyze(tree, this, treeType);
}
});
},
Expand Down
9 changes: 6 additions & 3 deletions packages/ember-auto-import/ts/splitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,13 +203,16 @@ export default class Splitter {
private chooseBundle(importedBy: Import[]) {
let usedInBundles = {} as { [bundleName: string]: boolean };
importedBy.forEach(usage => {
usedInBundles[this.bundleForPath(usage)] = true;
usedInBundles[this.bundleFor(usage)] = true;
});
return this.options.bundles.names.find(bundle => usedInBundles[bundle])!;
}

private bundleForPath(usage: Import) {
let bundleName = this.options.bundles.bundleForPath(usage.path);
private bundleFor(usage: Import) {
let bundleName = usage.treeType === undefined || typeof this.options.bundles.bundleForTreeType !== 'function'
? this.options.bundles.bundleForPath(usage.path)
: this.options.bundles.bundleForTreeType(usage.treeType)

if (this.options.bundles.names.indexOf(bundleName) === -1) {
throw new Error(
`bundleForPath("${
Expand Down
22 changes: 13 additions & 9 deletions packages/ember-auto-import/ts/tests/analyzer-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { UnwatchedDir } from 'broccoli-source';
import quickTemp from 'quick-temp';
import { ensureDirSync, readFileSync, outputFileSync, removeSync, existsSync } from 'fs-extra';
import { join } from 'path';
import Package from '../package';
import type Package from "../package";
import Analyzer from '../analyzer';

const { module: Qmodule, test } = QUnit;
Expand All @@ -14,7 +14,7 @@ Qmodule('analyzer', function(hooks) {
let builder: Builder;
let upstream: string;
let analyzer: Analyzer;
let pack: Partial<Package>;
let pack: Package;
let babelOptionsWasAccessed = false;

hooks.beforeEach(function(this: any) {
Expand All @@ -26,9 +26,9 @@ Qmodule('analyzer', function(hooks) {
return {};
},
babelMajorVersion: 6,
fileExtensions: ['js']
};
analyzer = new Analyzer(new UnwatchedDir(upstream), pack as Package);
fileExtensions: ["js"],
} as Package;
analyzer = new Analyzer(new UnwatchedDir(upstream), pack);
builder = new broccoli.Builder(analyzer);
});

Expand Down Expand Up @@ -96,7 +96,8 @@ Qmodule('analyzer', function(hooks) {
isDynamic: false,
specifier: 'some-package',
path: 'sample.js',
package: pack
package: pack,
treeType: undefined,
}]);
});

Expand All @@ -113,7 +114,8 @@ Qmodule('analyzer', function(hooks) {
isDynamic: false,
specifier: 'some-package',
path: 'sample.js',
package: pack
package: pack,
treeType: undefined,
}]);
});

Expand All @@ -130,12 +132,14 @@ Qmodule('analyzer', function(hooks) {
isDynamic: false,
specifier: 'some-package',
path: 'sample.js',
package: pack
package: pack,
treeType: undefined,
},{
isDynamic: false,
specifier: 'other-package',
path: 'sample.js',
package: pack
package: pack,
treeType: undefined,
}]);
});

Expand Down

0 comments on commit 07ecb9a

Please sign in to comment.