Skip to content
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

linkto routable engine path in host app #1443

Merged
merged 12 commits into from
May 30, 2023
6 changes: 4 additions & 2 deletions packages/compat/src/audit/babel-visitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export function auditJS(rawSource: string, filename: string, babelConfig: Transf
},
CallExpression(path: NodePath<t.CallExpression>) {
let callee = path.get('callee');
if (callee.referencesImport('@embroider/macros', 'importSync') || t.isImport(callee)) {
if (callee.referencesImport('@embroider/macros', 'importSync') || t.isImport(callee.node)) {
let arg = path.node.arguments[0];
if (arg.type === 'StringLiteral') {
imports.push({
Expand All @@ -69,7 +69,9 @@ export function auditJS(rawSource: string, filename: string, babelConfig: Transf
});
} else {
problems.push({
message: `audit tool is unable to understand this usage of ${t.isImport(callee) ? 'import' : 'importSync'}`,
message: `audit tool is unable to understand this usage of ${
t.isImport(callee.node) ? 'import' : 'importSync'
}`,
detail: arg.type,
codeFrameIndex: saveCodeFrame(arg),
});
Expand Down
17 changes: 15 additions & 2 deletions packages/compat/src/v1-addon.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Memoize } from 'typescript-memoize';
import { dirname, join, relative } from 'path';
import { dirname, join, relative, resolve } from 'path';
import { sync as pkgUpSync } from 'pkg-up';
import { existsSync, pathExistsSync } from 'fs-extra';
import buildFunnel, { Options as FunnelOptions } from 'broccoli-funnel';
Expand Down Expand Up @@ -732,7 +732,20 @@ export default class V1Addon {
// .hbs.js
templateExtensions: ['.hbs', '.hbs.js'],
});
if (!this.addonOptions.staticAddonTrees) {
if (this.addonOptions.staticAddonTrees) {
if (this.isEngine()) {
// even when staticAddonTrees is enabled, engines may have a router map
// that needs to be dynamically resolved.
let hasRoutesModule = false;

tree = new ObserveTree(tree, outputDir => {
hasRoutesModule = existsSync(resolve(outputDir, 'routes.js'));
});
built.dynamicMeta.push(() => ({
'implicit-modules': hasRoutesModule ? ['./routes.js'] : [],
}));
}
} else {
let filenames: string[] = [];
let templateOnlyComponentNames: string[] = [];

Expand Down
4 changes: 3 additions & 1 deletion packages/core/src/module-resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,9 @@ export class Resolver {
// the app is always the first engine
return this.options.engines[0];
}
let owningEngine = this.options.engines.find(e => e.activeAddons.find(a => a.root === pkg.root));
let owningEngine = this.options.engines.find(e =>
pkg.isEngine() ? e.root === pkg.root : e.activeAddons.find(a => a.root === pkg.root)
);
if (!owningEngine) {
throw new Error(
`bug in @embroider/core/src/module-resolver: cannot figure out the owning engine for ${pkg.root}`
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<nav>
<LinkTo @route="use-eager-engine">Eager</LinkTo>
<LinkTo @route="use-lazy-engine">Lazy</LinkTo>
<LinkTo @route="use-lazy-engine.inner-engine">Inner</LinkTo>
<LinkTo @route="style-check">Style Check</LinkTo>
</nav>

Expand Down
4 changes: 2 additions & 2 deletions tests/fixtures/engines-host-app/ember-cli-build.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,6 @@ module.exports = function (defaults) {
return app.toTree();
}

const Webpack = require('@embroider/webpack').Webpack;
return require('@embroider/compat').compatBuild(app, Webpack);
const { maybeEmbroider } = require('@embroider/test-setup');
return maybeEmbroider(app);
};
72 changes: 44 additions & 28 deletions tests/fixtures/engines-host-app/tests/acceptance/basics-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,24 +16,8 @@ function arrayOfCSSRules(styleSheets, cssSelector, cssProperty) {

return values.sort();
}

// We don't yet support lazy CSS in apps that are using fastboot. This test
// application runs both with and without fastboot.
const ensureCSSisLazy = !dependencySatisfies('ember-cli-fastboot', '*');

module('Acceptance | basics', function (hooks) {
setupApplicationTest(hooks);

test('host-app', async function (assert) {
await visit('/');
assert.equal(currentURL(), '/');
assert.dom('[data-test-duplicated-helper]').containsText('from-engines-host-app');
});

// this test must be the first test that loads the use-lazy-engine engine
// as after it has loaded it will not "unload" and we are checking that these
// modules are entering require.entries for the first time.
test('lazy-engine', async function (assert) {
function createLazyEngineTest(type) {
return async function (assert) {
await visit('/');
let entriesBefore = Object.entries(window.require.entries).length;
let rules = arrayOfCSSRules(document.styleSheets, '.shared-style-target', 'content');
Expand Down Expand Up @@ -77,7 +61,11 @@ module('Acceptance | basics', function (hooks) {

await visit('/use-lazy-engine');
let entriesAfter = Object.entries(window.require.entries).length;
assert.ok(!!window.require.entries['lazy-engine/helpers/duplicated-helper']);
if (type === 'safe') {
assert.ok(!!window.require.entries['lazy-engine/helpers/duplicated-helper']);
} else {
assert.notOk(!!window.require.entries['lazy-engine/helpers/duplicated-helper']);
}
assert.ok(entriesAfter > entriesBefore);
assert.equal(currentURL(), '/use-lazy-engine');
assert.dom('[data-test-lazy-engine-main] > h1').containsText('Lazy engine');
Expand Down Expand Up @@ -118,15 +106,11 @@ module('Acceptance | basics', function (hooks) {
'2px',
'lazy-engine vendor styles are present'
);
});

// See TODO comment in above test
//skip('lazy engines own app tree is lazy', function () {});
};
}

// this test must be the first test that loads the lazy-in-repo-engine as after it has loaded
// it will not "unload" and we are checkign that these modules are entering require.entries
// for the first time.
test('lazy-in-repo-engine', async function (assert) {
function createLazyInRepoEngineTest(type) {
return async function (assert) {
await visit('/');
const entriesBefore = Object.entries(window.require.entries).length;
let rules = arrayOfCSSRules(document.styleSheets, '.shared-style-target', 'content');
Expand Down Expand Up @@ -154,7 +138,11 @@ module('Acceptance | basics', function (hooks) {

await visit('/use-lazy-in-repo-engine');
const entriesAfter = Object.entries(window.require.entries).length;
assert.ok(!!window.require.entries['lazy-in-repo-engine/helpers/duplicated-helper']);
if (type === 'safe') {
assert.ok(!!window.require.entries['lazy-in-repo-engine/helpers/duplicated-helper']);
} else {
assert.notOk(!!window.require.entries['lazy-in-repo-engine/helpers/duplicated-helper']);
}
assert.ok(entriesAfter > entriesBefore);
assert.equal(currentURL(), '/use-lazy-in-repo-engine');
assert.dom('[data-test-lazy-in-repo-engine-main] > h1').containsText('Lazy In-Repo Engine');
Expand All @@ -177,8 +165,36 @@ module('Acceptance | basics', function (hooks) {
'2px',
'lazy-in-repo-engine addon styles are present'
);
};
}
// We don't yet support lazy CSS in apps that are using fastboot. This test
// application runs both with and without fastboot.
const ensureCSSisLazy = !dependencySatisfies('ember-cli-fastboot', '*');

module('Acceptance | basics', function (hooks) {
setupApplicationTest(hooks);

test('host-app', async function (assert) {
await visit('/');
assert.equal(currentURL(), '/');
assert.dom('[data-test-duplicated-helper]').containsText('from-engines-host-app');
});

// this test must be the first test that loads the use-lazy-engine engine
// as after it has loaded it will not "unload" and we are checking that these
// modules are entering require.entries for the first time.
test('@safe lazy-engine', createLazyEngineTest('safe'));
test('@optimized lazy-engine', createLazyEngineTest('optimized'));

// See TODO comment in above test
//skip('lazy engines own app tree is lazy', function () {});

// this test must be the first test that loads the lazy-in-repo-engine as after it has loaded
// it will not "unload" and we are checkign that these modules are entering require.entries
// for the first time.
test('@safe lazy-in-repo-engine', createLazyInRepoEngineTest('safe'));
test('@optimized lazy-in-repo-engine', createLazyInRepoEngineTest('optimized'));

test('eager-engine', async function (assert) {
await visit('/use-eager-engine');
assert.equal(currentURL(), '/use-eager-engine');
Expand Down
4 changes: 3 additions & 1 deletion tests/fixtures/lazy-engine/addon/routes.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import buildRoutes from 'ember-engines/routes';

export default buildRoutes(function() {});
export default buildRoutes(function() {
this.route('inner-engine')
});
39 changes: 35 additions & 4 deletions tests/scenarios/engines-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,23 @@ engineScenarios
expectFile = expectFilesAt(readFileSync(join(app.dir, 'dist/.stage2-output'), 'utf8'), { qunit: assert });
});

test(`pnpm test`, async function (assert) {
let result = await app.execute('pnpm test');
test(`pnpm test safe`, async function (assert) {
let result = await app.execute('pnpm test --filter=!@optimized', {
env: {
EMBROIDER_TEST_SETUP_OPTIONS: 'safe',
EMBROIDER_TEST_SETUP_FORCE: 'embroider',
},
});
assert.equal(result.exitCode, 0, result.output);
});

test(`pnpm test optimized`, async function (assert) {
let result = await app.execute('pnpm test --filter=!@safe', {
env: {
EMBROIDER_TEST_SETUP_OPTIONS: 'optimized',
EMBROIDER_TEST_SETUP_FORCE: 'embroider',
},
});
assert.equal(result.exitCode, 0, result.output);
});

Expand All @@ -117,6 +132,8 @@ engineScenarios
});

engineScenarios
.skip('lts_3_28-engines') // fails due to https://github.com/emberjs/ember.js/pull/20461
.skip('lts_4_4-engines') // fails due to https://github.com/emberjs/ember.js/pull/20461
.skip('release-engines') // fails due to https://github.com/emberjs/ember.js/pull/20461
.map('with-fastboot', app => {
app.linkDependency('ember-cli-fastboot', { baseDir: __dirname });
Expand All @@ -130,11 +147,25 @@ engineScenarios
app = await scenario.prepare();
});

test(`pnpm test`, async function (assert) {
let result = await app.execute('pnpm test');
test(`pnpm test safe`, async function (assert) {
let result = await app.execute('pnpm test --filter=!@optimized', {
env: {
EMBROIDER_TEST_SETUP_OPTIONS: 'safe',
EMBROIDER_TEST_SETUP_FORCE: 'embroider',
},
});
assert.equal(result.exitCode, 0, result.output);
});

test(`pnpm test optimized`, async function (assert) {
let result = await app.execute('pnpm test --filter=!@safe', {
env: {
EMBROIDER_TEST_SETUP_OPTIONS: 'optimized',
EMBROIDER_TEST_SETUP_FORCE: 'embroider',
},
});
assert.equal(result.exitCode, 0, result.output);
});
let visit: any;

hooks.before(async () => {
Expand Down