Skip to content

Commit

Permalink
Detect addons with customized treeForMethod names
Browse files Browse the repository at this point in the history
The normal way for addons to implement custom treeFor hooks is to... just implement them.

But some addons mutate the `treeForMethods` table instead. In fact, some addons mutate *other addons* `treeForMethods` table. *sigh*
  • Loading branch information
ef4 committed Jul 4, 2022
1 parent 3606af0 commit 6b13a79
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 1 deletion.
25 changes: 24 additions & 1 deletion packages/compat/src/v1-addon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,19 @@ const dynamicTreeHooks = Object.freeze([
'treeForVendor',
]);

const defaultMethods = {
app: 'treeForApp',
addon: 'treeForAddon',
'addon-styles': 'treeForAddonStyles',
'addon-templates': 'treeForAddonTemplates',
'addon-test-support': 'treeForAddonTestSupport',
public: 'treeForPublic',
styles: 'treeForStyles',
templates: 'treeForTemplates',
'test-support': 'treeForTestSupport',
vendor: 'treeForVendor',
};

const appPublicationDir = '_app_';
const fastbootPublicationDir = '_fastboot_';

Expand Down Expand Up @@ -400,12 +413,22 @@ export default class V1Addon {
// customized hook exists in actual code exported from their index.js
this.mainModule[treeName] ||
// addon instance doesn't match its own prototype
(realAddon.__proto__ && realAddon[treeName] !== realAddon.__proto__[treeName])
(realAddon.__proto__ && realAddon[treeName] !== realAddon.__proto__[treeName]) ||
this.customizesHookName(treeName)
);
})
);
}

private customizesHookName(treeName: string): boolean {
for (let [name, methodName] of Object.entries(defaultMethods)) {
if (methodName === treeName) {
return this.addonInstance.treeForMethods?.[name] !== methodName;
}
}
return false;
}

@Memoize()
private hasStockTree(treeName: AddonTreePath): boolean {
if (this.suppressesTree(treeName)) {
Expand Down
1 change: 1 addition & 0 deletions packages/shared-internals/src/ember-cli-models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ interface BaseAddonInstance {
public: string;
vendor: string;
};
treeForMethods: Record<string, string>;
}

export type AddonTreePath = keyof BaseAddonInstance['treePaths'];
Expand Down
25 changes: 25 additions & 0 deletions tests/scenarios/stage1-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,26 @@ appScenarios
externallyCustomized.pkg.name = 'externally-customized';
troubleMaker.pkg.name = 'trouble-maker';

// an addon that customizes a tree by mutating treeForMethods
let patchesMethodName = baseAddon();
merge(patchesMethodName.files, {
injected: {
hello: { 'world.js': '// hello' },
},
'index.js': `
const { join } = require('path');
module.exports = {
name: 'patches-method-name',
included() {
this.treeForMethods['addon'] = 'notTheUsual';
},
notTheUsual() {
return join(this.root, 'injected');
}
}`,
});
patchesMethodName.pkg.name = 'patches-method-name';

// an addon that customizes packageJSON['ember-addon'].main and then uses
// stock trees. Setting the main actually changes the root for *all* stock
// trees.
Expand Down Expand Up @@ -346,6 +366,7 @@ appScenarios
project.addDependency(movedMain);
project.addDependency(suppressed);
project.addDependency(suppressedCustom);
project.addDependency(patchesMethodName);

project.pkg['ember-addon'] = { paths: ['lib/disabled-in-repo-addon', 'lib/blacklisted-in-repo-addon'] };
merge(project.files, loadFromFixtureData('blacklisted-addon-build-options'));
Expand Down Expand Up @@ -380,6 +401,10 @@ appScenarios
assert.ok(fs.existsSync(join(workspaceDir, 'node_modules/externally-customized/public/hello/world.js')));
});

test('custom tree hooks are detected when they have been customized via treeForMethod names', function (assert) {
assert.ok(fs.existsSync(join(workspaceDir, 'node_modules/patches-method-name/hello/world.js')));
});

test('addon with customized ember-addon.main can still use stock trees', function (assert) {
let fileContents = fs.readFileSync(join(workspaceDir, 'node_modules/moved-main/helpers/hello.js'));
assert.ok(/hello-world/.test(fileContents.toString()));
Expand Down

0 comments on commit 6b13a79

Please sign in to comment.