Skip to content

Commit

Permalink
Merge pull request #209 from embroider-build/relocation
Browse files Browse the repository at this point in the history 
Handle appTree relocation for all deps
  • Loading branch information
@ef4
ef4 authored Jun 4, 2019
2 parents 5ad0567 + 41bb375 commit b81ae1a
Show file tree
Hide file tree
Showing 7 changed files with 373 additions and 59 deletions.
20 changes: 20 additions & 0 deletions packages/compat/src/dependency-rules.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,24 @@ export interface ComponentRules {
//
yieldsSafeComponents?: (boolean | { [name: string]: boolean })[];

// This declares that our component yields some of its arguments unchanged.
//
// The array corresponds to your yielded positional arguments. Each value can
// be:
// false, meaning this yielded value is not one of our arguments
// a string, meaning this yielded value is our argument with that name
// or a POJO, whose individual properties are string naming which arguments
// from whence they came.
//
// Examples:
//
// If you do: {{yield @foo}}
// Then say: yieldsArguments: ['foo']
//
// If you do: {{yield (hash x=@foo) }}
// Then say: yieldsArguments: [{ x: 'foo' }]
yieldsArguments?: (string | { [name: string]: string })[];

// This declares that our component accepts arguments that will be invoked
// with the {{component}} helper. This silences warnings in the places where
// we consume them, while introducing warnings in the places where people are
Expand Down Expand Up @@ -113,6 +131,7 @@ type ComponentSnippet = string;

export interface PreprocessedComponentRule {
yieldsSafeComponents: Required<ComponentRules>['yieldsSafeComponents'];
yieldsArguments: Required<ComponentRules>['yieldsArguments'];
argumentsAreComponents: string[];
safeInteriorPaths: string[];
}
Expand Down Expand Up @@ -142,6 +161,7 @@ export function preprocessComponentRule(componentRules: ComponentRules): Preproc
argumentsAreComponents,
safeInteriorPaths,
yieldsSafeComponents: componentRules.yieldsSafeComponents || [],
yieldsArguments: componentRules.yieldsArguments || [],
};
}

Expand Down
107 changes: 61 additions & 46 deletions packages/compat/src/resolver-transform.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { default as Resolver } from './resolver';
import { ComponentRules } from './dependency-rules';
import { default as Resolver, ComponentResolution } from './resolver';

// This is the AST transform that resolves components and helpers at build time
// and puts them into `dependencies`.
Expand All @@ -15,14 +14,10 @@ export function makeResolverTransform(resolver: Resolver) {
visitor: {
Program: {
enter(node: any) {
if (node.blockParams.length > 0) {
scopeStack.push(node.blockParams);
}
scopeStack.push(node.blockParams);
},
exit(node: any) {
if (node.blockParams.length > 0) {
scopeStack.pop();
}
exit() {
scopeStack.pop();
},
},
BlockStatement(node: any) {
Expand All @@ -39,17 +34,10 @@ export function makeResolverTransform(resolver: Resolver) {
// a block counts as args from our perpsective (it's enough to prove
// this thing must be a component, not content)
let hasArgs = true;
let resolution = resolver.resolveMustache(node.path.original, hasArgs, env.moduleName);
if (resolution) {
if (
resolution.type === 'component' &&
node.program.blockParams.length > 0 &&
resolution.yieldsComponents.length > 0
) {
scopeStack.yieldingComponents(resolution.yieldsComponents);
}
if (resolution.type === 'component') {
for (let name of resolution.argumentsAreComponents) {
const resolution = resolver.resolveMustache(node.path.original, hasArgs, env.moduleName);
if (resolution && resolution.type === 'component') {
scopeStack.enteringComponentBlock(resolution, ({ argumentsAreComponents }) => {
for (let name of argumentsAreComponents) {
let pair = node.hash.pairs.find((pair: any) => pair.key === name);
if (pair) {
handleImpliedComponentHelper(
Expand All @@ -62,7 +50,7 @@ export function makeResolverTransform(resolver: Resolver) {
);
}
}
}
});
}
},
SubExpression(node: any) {
Expand Down Expand Up @@ -110,27 +98,22 @@ export function makeResolverTransform(resolver: Resolver) {
ElementNode: {
enter(node: any) {
if (!scopeStack.inScope(node.tag.split('.')[0])) {
let resolution = resolver.resolveElement(node.tag, env.moduleName);
const resolution = resolver.resolveElement(node.tag, env.moduleName);
if (resolution && resolution.type === 'component') {
if (node.blockParams.length > 0 && resolution.yieldsComponents.length > 0) {
scopeStack.yieldingComponents(resolution.yieldsComponents);
}
for (let name of resolution.argumentsAreComponents) {
let attr = node.attributes.find((attr: any) => attr.name === '@' + name);
if (attr) {
handleImpliedComponentHelper(node.tag, name, attr.value, resolver, env.moduleName, scopeStack);
scopeStack.enteringComponentBlock(resolution, ({ argumentsAreComponents }) => {
for (let name of argumentsAreComponents) {
let attr = node.attributes.find((attr: any) => attr.name === '@' + name);
if (attr) {
handleImpliedComponentHelper(node.tag, name, attr.value, resolver, env.moduleName, scopeStack);
}
}
}
});
}
}
if (node.blockParams.length > 0) {
scopeStack.push(node.blockParams);
}
scopeStack.push(node.blockParams);
},
exit(node: any) {
if (node.blockParams.length > 0) {
scopeStack.pop();
}
exit() {
scopeStack.pop();
},
},
},
Expand All @@ -144,9 +127,14 @@ export function makeResolverTransform(resolver: Resolver) {
return resolverTransform;
}

type ScopeEntry =
| { type: 'blockParams'; blockParams: string[] }
| { type: 'safeComponentMarker'; safeComponentMarker: Required<ComponentRules>['yieldsSafeComponents'] };
interface ComponentBlockMarker {
type: 'componentBlockMarker';
resolution: ComponentResolution;
argumentsAreComponents: string[];
exit: (marker: ComponentBlockMarker) => void;
}

type ScopeEntry = { type: 'blockParams'; blockParams: string[] } | ComponentBlockMarker;

class ScopeStack {
private stack: ScopeEntry[] = [];
Expand All @@ -161,16 +149,23 @@ class ScopeStack {
// by a safe component marker, we also clear that.
pop() {
this.stack.shift();
if (this.stack.length > 0 && this.stack[0]!.type === 'safeComponentMarker') {
let next = this.stack[0];
if (next && next.type === 'componentBlockMarker') {
next.exit(next);
this.stack.shift();
}
}

// right before we enter a block, we might determine that some of the values
// that will be yielded as marked (by a rule) as safe to be used with the
// {{component}} helper.
yieldingComponents(safeComponentMarker: Required<ComponentRules>['yieldsSafeComponents']) {
this.stack.unshift({ type: 'safeComponentMarker', safeComponentMarker });
enteringComponentBlock(resolution: ComponentResolution, exit: ComponentBlockMarker['exit']) {
this.stack.unshift({
type: 'componentBlockMarker',
resolution,
argumentsAreComponents: resolution.argumentsAreComponents.slice(),
exit,
});
}

inScope(name: string) {
Expand All @@ -193,19 +188,39 @@ class ScopeStack {
for (let i = 0; i < this.stack.length - 1; i++) {
let here = this.stack[i];
let next = this.stack[i + 1];
if (here.type === 'blockParams' && next.type === 'safeComponentMarker') {
if (here.type === 'blockParams' && next.type === 'componentBlockMarker') {
let positionalIndex = here.blockParams.indexOf(parts[0]);
if (positionalIndex === -1) {
continue;
}

if (parts.length === 1) {
return next.safeComponentMarker[positionalIndex] === true;
if (next.resolution.yieldsComponents[positionalIndex] === true) {
return true;
}
let sourceArg = next.resolution.yieldsArguments[positionalIndex];
if (typeof sourceArg === 'string') {
next.argumentsAreComponents.push(sourceArg);
return true;
}
} else {
let entry = next.safeComponentMarker[positionalIndex];
let entry = next.resolution.yieldsComponents[positionalIndex];
if (entry && typeof entry === 'object') {
return entry[parts[1]] === true;
}

let argsEntry = next.resolution.yieldsArguments[positionalIndex];
if (argsEntry && typeof argsEntry === 'object') {
let sourceArg = argsEntry[parts[1]];
if (typeof sourceArg === 'string') {
next.argumentsAreComponents.push(sourceArg);
return true;
}
}
}
// we found the source of the name, but there were no rules to cover it.
// Don't keep searching higher, those are different names.
return false;
}
}
return false;
Expand Down
28 changes: 16 additions & 12 deletions packages/compat/src/resolver.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,22 @@ import { Memoize } from 'typescript-memoize';
import { ResolvedDep } from '@embroider/core/src/resolver';
import resolve from 'resolve';

type ResolutionResult =
| {
type: 'component';
modules: ResolvedDep[];
yieldsComponents: Required<ComponentRules>['yieldsSafeComponents'];
argumentsAreComponents: string[];
}
| {
type: 'helper';
modules: ResolvedDep[];
};
export interface ComponentResolution {
type: 'component';
modules: ResolvedDep[];
yieldsComponents: Required<ComponentRules>['yieldsSafeComponents'];
yieldsArguments: Required<ComponentRules>['yieldsArguments'];
argumentsAreComponents: string[];
}

export interface HelperResolution {
type: 'helper';
modules: ResolvedDep[];
}

export type ResolutionResult = ComponentResolution | HelperResolution;

interface ResolutionFail {
export interface ResolutionFail {
type: 'error';
hardFail: boolean;
message: string;
Expand Down Expand Up @@ -324,6 +327,7 @@ export default class CompatResolver implements Resolver {
runtimeName: p.runtimeName,
})),
yieldsComponents: componentRules ? componentRules.yieldsSafeComponents : [],
yieldsArguments: componentRules ? componentRules.yieldsArguments : [],
argumentsAreComponents: componentRules ? componentRules.argumentsAreComponents : [],
};
}
Expand Down
12 changes: 12 additions & 0 deletions packages/compat/tests/renaming-tests.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,14 @@ QUnit.module('renaming tests', function(origHooks) {
(firstAddonWithAppTreeImport.files.app as Project['files'])[
'first.js'
] = `export { default } from 'has-app-tree-import';`;
(firstAddonWithAppTreeImport.files.app as Project['files'])[
'imports-dep.js'
] = `export { default } from 'inner-dep';`;
(firstAddonWithAppTreeImport.files.addon as Project['files'])['index.js'] = `export default "first-copy";`;

let innerDep = firstAddonWithAppTreeImport.addAddon('inner-dep');
(innerDep.files.addon as Project['files'])['index.js'] = `export default "inner-dep";`;

let secondAddonWithAppTreeImport = app.addAddon('intermediate').addAddon('has-app-tree-import');
(secondAddonWithAppTreeImport.files.app as Project['files'])[
'second.js'
Expand Down Expand Up @@ -168,4 +174,10 @@ QUnit.module('renaming tests', function(origHooks) {
/export \{ default \} from ['"]\.\/node_modules\/intermediate\/node_modules\/has-app-tree-import['"]/
);
});
test(`files copied into app from addons resolve the addon's deps`, function(assert) {
let assertFile = assert.file('imports-dep.js').transform(build.transpile);
assertFile.matches(
/export \{ default \} from ['"]\.\/node_modules\/has-app-tree-import\/node_modules\/inner-dep['"]/
);
});
});
Loading

0 comments on commit b81ae1a

Please sign in to comment.