From 98bb89796c5a19a69606e51cc8307e79e316fbff Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Thu, 24 Feb 2022 17:46:11 -0500 Subject: [PATCH 1/8] extract a reusable v2-addon-template --- package.json | 3 +- tests/scenarios/scenarios.ts | 4 +++ tests/scenarios/v2-addon-test.ts | 50 ++++++++++----------------- tests/v2-addon-template/addon-main.js | 2 ++ tests/v2-addon-template/package.json | 16 +++++++++ 5 files changed, 42 insertions(+), 33 deletions(-) create mode 100644 tests/v2-addon-template/addon-main.js create mode 100644 tests/v2-addon-template/package.json diff --git a/package.json b/package.json index 0c3b39b46..03c9309d5 100644 --- a/package.json +++ b/package.json @@ -16,7 +16,8 @@ "types/ember-cli-htmlbars", "tests/scenarios", "tests/app-template", - "tests/addon-template" + "tests/addon-template", + "tests/v2-addon-template" ], "nohoist": [ "**/@types/broccoli-plugin" diff --git a/tests/scenarios/scenarios.ts b/tests/scenarios/scenarios.ts index cb8868a4f..260a86147 100644 --- a/tests/scenarios/scenarios.ts +++ b/tests/scenarios/scenarios.ts @@ -65,6 +65,10 @@ export function baseAddon(as: 'dummy-app' | 'dependency' = 'dependency') { ); } +export function baseV2Addon() { + return Project.fromDir(dirname(require.resolve('../v2-addon-template/package.json')), { linkDeps: true }); +} + export function baseApp() { return Project.fromDir(dirname(require.resolve('../app-template/package.json')), { linkDevDeps: true }); } diff --git a/tests/scenarios/v2-addon-test.ts b/tests/scenarios/v2-addon-test.ts index 77ae7df5c..4788210fc 100644 --- a/tests/scenarios/v2-addon-test.ts +++ b/tests/scenarios/v2-addon-test.ts @@ -1,41 +1,28 @@ -import { appScenarios } from './scenarios'; -import { PreparedApp, Project } from 'scenario-tester'; +import { appScenarios, baseV2Addon } from './scenarios'; +import { PreparedApp } from 'scenario-tester'; import QUnit from 'qunit'; import merge from 'lodash/merge'; -import { AddonMeta } from '@embroider/shared-internals'; + const { module: Qmodule, test } = QUnit; appScenarios .map('v2-addon', project => { - let meta: AddonMeta = { - type: 'addon', - version: 2, - 'app-js': { - './components/example-component.js': 'app/components/example-component.js', - }, - main: 'addon-main.js', - }; - - let packageJSON = { - keywords: ['ember-addon'], - 'ember-addon': meta, - }; - - let addon = new Project({ - name: 'v2-addon', - files: { - 'package.json': JSON.stringify(packageJSON, null, 2), - app: { - components: { - 'example-component.js': `export { default } from 'v2-addon/components/example-component';`, - }, + let addon = baseV2Addon(); + addon.pkg.name = 'v2-addon'; + (addon.pkg as any)['ember-addon']['app-js']['./components/example-component.js'] = + 'app/components/example-component.js'; + merge(addon.files, { + app: { + components: { + 'example-component.js': `export { default } from 'v2-addon/components/example-component';`, }, - 'addon-main.js': ` + }, + 'addon-main.js': ` const { addonV1Shim } = require('@embroider/addon-shim'); module.exports = addonV1Shim(__dirname); `, - components: { - 'example-component.js': ` + components: { + 'example-component.js': ` import Component from '@glimmer/component'; import { hbs } from 'ember-cli-htmlbars'; import { setComponentTemplate } from '@ember/component'; @@ -45,16 +32,15 @@ appScenarios } setComponentTemplate(TEMPLATE, ExampleComponent); `, - }, - 'import-from-npm.js': ` + }, + 'import-from-npm.js': ` export default async function() { let { message } = await import('third-party'); return message() } `, - }, }); - addon.linkDependency('@embroider/addon-shim', { baseDir: __dirname }); + addon.addDependency('third-party', { files: { 'index.js': ` diff --git a/tests/v2-addon-template/addon-main.js b/tests/v2-addon-template/addon-main.js new file mode 100644 index 000000000..9375da065 --- /dev/null +++ b/tests/v2-addon-template/addon-main.js @@ -0,0 +1,2 @@ +const { addonV1Shim } = require('@embroider/addon-shim'); +module.exports = addonV1Shim(__dirname); diff --git a/tests/v2-addon-template/package.json b/tests/v2-addon-template/package.json new file mode 100644 index 000000000..60d3fbd74 --- /dev/null +++ b/tests/v2-addon-template/package.json @@ -0,0 +1,16 @@ +{ + "name": "v2-addon-template", + "version": "0.0.0", + "keywords": [ + "ember-addon" + ], + "dependencies": { + "@embroider/addon-shim": "1.2.0" + }, + "ember-addon": { + "type": "addon", + "version": 2, + "app-js": {}, + "main": "addon-main.js" + } +} From 8c70a1457de94fab2f6b2dbfc1f7fa7d83efec08 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Thu, 24 Feb 2022 18:00:21 -0500 Subject: [PATCH 2/8] use embroider/test-setup in app-template So that we can use process.env.EMBROIDER_TEST_SETUP_OPTIONS to configure the apps. --- tests/app-template/ember-cli-build.js | 21 +++------------------ tests/app-template/package.json | 1 + 2 files changed, 4 insertions(+), 18 deletions(-) diff --git a/tests/app-template/ember-cli-build.js b/tests/app-template/ember-cli-build.js index 1d9b7e50e..ab01aed3e 100644 --- a/tests/app-template/ember-cli-build.js +++ b/tests/app-template/ember-cli-build.js @@ -1,27 +1,12 @@ 'use strict'; const EmberApp = require('ember-cli/lib/broccoli/ember-app'); +const { maybeEmbroider } = require('@embroider/test-setup'); module.exports = function (defaults) { - let app = new EmberApp(defaults, { - // Add options here - }); - - // Use `app.import` to add additional libraries to the generated - // output files. - // - // If you need to use different assets in different - // environments, specify an object as the first parameter. That - // object's keys should be the environment name and the values - // should be the asset to use in that environment. - // - // If the library that you are including contains AMD or ES6 - // modules that you would like to import into your application - // please specify an object with the list of modules as keys - // along with the exports of each module as its value. + let app = new EmberApp(defaults, {}); - const { Webpack } = require('@embroider/webpack'); - return require('@embroider/compat').compatBuild(app, Webpack, { + return maybeEmbroider(app, { skipBabel: [ { package: 'qunit', diff --git a/tests/app-template/package.json b/tests/app-template/package.json index ef1ee9af6..c5bc1dbef 100644 --- a/tests/app-template/package.json +++ b/tests/app-template/package.json @@ -29,6 +29,7 @@ "@embroider/compat": "1.2.0", "@embroider/core": "1.2.0", "@embroider/router": "1.2.0", + "@embroider/test-setup": "1.2.0", "@embroider/webpack": "1.2.0", "@glimmer/component": "^1.0.4", "@glimmer/tracking": "^1.0.4", From 2b28375a30cbb44da2f833981237f89a4c710fc1 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Thu, 24 Feb 2022 18:25:26 -0500 Subject: [PATCH 3/8] switch fastboot-app to use optimized settings --- tests/scenarios/fastboot-app-test.ts | 8 +++++++- tests/scenarios/helpers.ts | 6 ++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/scenarios/fastboot-app-test.ts b/tests/scenarios/fastboot-app-test.ts index 5317e5ea9..b3f6d600b 100644 --- a/tests/scenarios/fastboot-app-test.ts +++ b/tests/scenarios/fastboot-app-test.ts @@ -53,6 +53,8 @@ appScenarios let result = await app.execute(`yarn test`, { env: { EMBER_ENV: env, + EMBROIDER_TEST_SETUP_OPTIONS: 'optimized', + EMBROIDER_TEST_SETUP_FORCE: 'embroider', }, }); assert.equal(result.exitCode, 0, result.output); @@ -63,7 +65,11 @@ appScenarios let doc: any; hooks.before(async () => { - ({ visit } = await setupFastboot(app, env)); + ({ visit } = await setupFastboot(app, env, { + EMBER_ENV: env, + EMBROIDER_TEST_SETUP_OPTIONS: 'optimized', + EMBROIDER_TEST_SETUP_FORCE: 'embroider', + })); doc = (await visit('/')).window.document; }); diff --git a/tests/scenarios/helpers.ts b/tests/scenarios/helpers.ts index 9cd3e14aa..850c11dca 100644 --- a/tests/scenarios/helpers.ts +++ b/tests/scenarios/helpers.ts @@ -4,8 +4,10 @@ import { readFileSync } from 'fs'; import globby from 'globby'; import { set } from 'lodash'; -export async function setupFastboot(app: PreparedApp, environment = 'development') { - let result = await app.execute(`node node_modules/ember-cli/bin/ember build --environment=${environment}`); +export async function setupFastboot(app: PreparedApp, environment = 'development', envVars?: Record) { + let result = await app.execute(`node node_modules/ember-cli/bin/ember build --environment=${environment}`, { + env: envVars, + }); if (result.exitCode !== 0) { throw new Error(`failed to build app for fastboot: ${result.output}`); From 1151960df681e37952a77c1a668bd4ecb2f08654 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Thu, 3 Mar 2022 00:37:07 -0500 Subject: [PATCH 4/8] add test coverage for eager css in fastboot --- .../fastboot-app/app/templates/index.hbs | 5 +- .../tests/acceptance/basic-test.js | 9 +++ tests/scenarios/fastboot-app-test.ts | 68 +++++++++++++++---- tests/scenarios/helpers.ts | 23 ++++++- tests/scenarios/v2-addon-test.ts | 30 ++++---- 5 files changed, 101 insertions(+), 34 deletions(-) diff --git a/tests/fixtures/fastboot-app/app/templates/index.hbs b/tests/fixtures/fastboot-app/app/templates/index.hbs index 246820600..d6d4267ad 100644 --- a/tests/fixtures/fastboot-app/app/templates/index.hbs +++ b/tests/fixtures/fastboot-app/app/templates/index.hbs @@ -1,5 +1,6 @@ -
Hello from fastboot-app
+
Hello from fastboot-app
- \ No newline at end of file + + \ No newline at end of file diff --git a/tests/fixtures/fastboot-app/tests/acceptance/basic-test.js b/tests/fixtures/fastboot-app/tests/acceptance/basic-test.js index fe25d5832..b3a540323 100644 --- a/tests/fixtures/fastboot-app/tests/acceptance/basic-test.js +++ b/tests/fixtures/fastboot-app/tests/acceptance/basic-test.js @@ -39,4 +39,13 @@ module('Acceptance | runtime basics', function (hooks) { test('the tests suite eagerly loads some code that the app uses only lazily', async function (assert) { assert.equal(secondSampleLib(), 'From second-sample-lib'); }); + + test('a component from a v2 addon with css', async function (assert) { + assert.dom('[data-test-v2-example]').containsText('it worked'); + assert.equal( + getComputedStyle(document.querySelector('[data-test-v2-example]')).color, + 'rgb(0, 128, 0)', + 'style was applied' + ); + }); }); diff --git a/tests/scenarios/fastboot-app-test.ts b/tests/scenarios/fastboot-app-test.ts index b3f6d600b..fbe19951a 100644 --- a/tests/scenarios/fastboot-app-test.ts +++ b/tests/scenarios/fastboot-app-test.ts @@ -1,8 +1,9 @@ -import { appScenarios, baseAddon } from './scenarios'; +import { appScenarios, baseAddon, baseV2Addon } from './scenarios'; import { PreparedApp, Project } from 'scenario-tester'; -import { setupFastboot, loadFromFixtureData } from './helpers'; +import { setupFastboot, loadFromFixtureData, FastbootTestHelpers } from './helpers'; import QUnit from 'qunit'; import merge from 'lodash/merge'; +import { JSDOM } from 'jsdom'; const { module: Qmodule, test } = QUnit; appScenarios @@ -27,6 +28,37 @@ appScenarios }) ); + let v2Example = baseV2Addon(); + v2Example.pkg.name = 'v2-example'; + (v2Example.pkg as any)['ember-addon']['app-js']['./components/v2-example-component.js'] = + 'app/components/v2-example-component.js'; + merge(v2Example.files, { + app: { + components: { + 'v2-example-component.js': `export { default } from 'v2-example/components/v2-example-component';`, + }, + }, + components: { + 'v2-example-component.js': ` + import Component from '@glimmer/component'; + import { hbs } from 'ember-cli-htmlbars'; + import { setComponentTemplate } from '@ember/component'; + import './v2-example-component.css'; + const TEMPLATE = hbs('
{{this.message}}
') + export default class ExampleComponent extends Component { + message = "it worked" + } + setComponentTemplate(TEMPLATE, ExampleComponent); + `, + 'v2-example-component.css': ` + [data-test-v2-example] { + color: green; + } + `, + }, + }); + project.addDependency(v2Example); + project.linkDependency('ember-cli-fastboot', { baseDir: __dirname }); project.linkDependency('fastboot', { baseDir: __dirname }); @@ -61,38 +93,48 @@ appScenarios }); Qmodule(`fastboot: ${env}`, function (hooks) { - let visit: any; - let doc: any; + let fb: FastbootTestHelpers; + let doc: JSDOM['window']['document']; hooks.before(async () => { - ({ visit } = await setupFastboot(app, env, { + fb = await setupFastboot(app, env, { EMBER_ENV: env, EMBROIDER_TEST_SETUP_OPTIONS: 'optimized', EMBROIDER_TEST_SETUP_FORCE: 'embroider', - })); - doc = (await visit('/')).window.document; + }); + doc = (await fb.visit('/')).window.document; }); test('content is rendered', async function (assert) { - assert.equal(doc.querySelector('[data-test="hello"]').textContent, 'Hello from fastboot-app'); + assert.equal(doc.querySelector('[data-test="hello"]')!.textContent, 'Hello from fastboot-app'); }); test('found server implementation of in-app module', async function (assert) { - assert.equal(doc.querySelector('[data-test="example"]').textContent, 'This is the server implementation'); + assert.equal(doc.querySelector('[data-test="example"]')!.textContent, 'This is the server implementation'); }); test('found server implementation of addon service', async function (assert) { - assert.equal(doc.querySelector('[data-test="addon-example"]').textContent, 'Server AddonExampleService'); + assert.equal(doc.querySelector('[data-test="addon-example"]')!.textContent, 'Server AddonExampleService'); }); test('found fastboot-only service from the app', async function (assert) { assert.equal( - doc.querySelector('[data-test="check-service"]').textContent.trim(), + doc.querySelector('[data-test="check-service"]')!.textContent!.trim(), `I'm a fastboot-only service in the app` ); }); test('found fastboot-only file from the addon', async function (assert) { - assert.equal(doc.querySelector('[data-test="check-addon-file"]').textContent.trim(), '42'); + assert.equal(doc.querySelector('[data-test="check-addon-file"]')!.textContent!.trim(), '42'); }); test('a component successfully lazy loaded some code', async function (assert) { - assert.equal(doc.querySelector('[data-test="lazy-component"]').textContent.trim(), 'From sample-lib'); + assert.equal(doc.querySelector('[data-test="lazy-component"]')!.textContent!.trim(), 'From sample-lib'); + }); + test('eager CSS from a v2 addon is present', async function (assert) { + for (let link of [...doc.querySelectorAll('link[rel="stylesheet"]')]) { + let src = await fb.fetchAsset(link.getAttribute('href')!); + if (/\[data-test-v2-example\]/.test(src)) { + assert.ok(true, 'found expected style'); + return; + } + } + assert.ok(false, 'did not find expected style'); }); }); }); diff --git a/tests/scenarios/helpers.ts b/tests/scenarios/helpers.ts index 850c11dca..e8af6adba 100644 --- a/tests/scenarios/helpers.ts +++ b/tests/scenarios/helpers.ts @@ -3,8 +3,18 @@ import { join } from 'path'; import { readFileSync } from 'fs'; import globby from 'globby'; import { set } from 'lodash'; +import type { JSDOM } from 'jsdom'; -export async function setupFastboot(app: PreparedApp, environment = 'development', envVars?: Record) { +export interface FastbootTestHelpers { + visit(url: string): Promise; + fetchAsset(url: string): Promise; +} + +export async function setupFastboot( + app: PreparedApp, + environment = 'development', + envVars?: Record +): Promise { let result = await app.execute(`node node_modules/ember-cli/bin/ember build --environment=${environment}`, { env: envVars, }); @@ -31,7 +41,16 @@ export async function setupFastboot(app: PreparedApp, environment = 'development return new JSDOM(html); } - return { visit }; + async function fetchAsset(url: string): Promise { + const origin = 'http://example.com'; + let u = new URL(url, origin); + if (u.origin !== origin) { + throw new Error(`fetchAsset only supports local assets, you asked for ${url}`); + } + return readFileSync(join(app.dir, 'dist', u.pathname), 'utf-8'); + } + + return { visit, fetchAsset }; } export function loadFromFixtureData(fixtureNamespace: string) { diff --git a/tests/scenarios/v2-addon-test.ts b/tests/scenarios/v2-addon-test.ts index 4788210fc..956931ed8 100644 --- a/tests/scenarios/v2-addon-test.ts +++ b/tests/scenarios/v2-addon-test.ts @@ -17,27 +17,23 @@ appScenarios 'example-component.js': `export { default } from 'v2-addon/components/example-component';`, }, }, - 'addon-main.js': ` - const { addonV1Shim } = require('@embroider/addon-shim'); - module.exports = addonV1Shim(__dirname); - `, components: { 'example-component.js': ` - import Component from '@glimmer/component'; - import { hbs } from 'ember-cli-htmlbars'; - import { setComponentTemplate } from '@ember/component'; - const TEMPLATE = hbs('
{{this.message}}
') - export default class ExampleComponent extends Component { - message = "it worked" - } - setComponentTemplate(TEMPLATE, ExampleComponent); - `, + import Component from '@glimmer/component'; + import { hbs } from 'ember-cli-htmlbars'; + import { setComponentTemplate } from '@ember/component'; + const TEMPLATE = hbs('
{{this.message}}
') + export default class ExampleComponent extends Component { + message = "it worked" + } + setComponentTemplate(TEMPLATE, ExampleComponent); + `, }, 'import-from-npm.js': ` - export default async function() { - let { message } = await import('third-party'); - return message() - } + export default async function() { + let { message } = await import('third-party'); + return message() + } `, }); From 79475574ddc58d16101fb8a4b11c8857d50a8f1a Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Thu, 3 Mar 2022 22:30:26 -0500 Subject: [PATCH 5/8] test coverate for lazy css --- .../fixtures/fastboot-app/app/routes/index.js | 4 ++++ .../tests/acceptance/basic-test.js | 11 ++++++++++- tests/scenarios/fastboot-app-test.ts | 19 +++++++++++++++++-- 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/tests/fixtures/fastboot-app/app/routes/index.js b/tests/fixtures/fastboot-app/app/routes/index.js index 47107cd4b..99faa15fd 100644 --- a/tests/fixtures/fastboot-app/app/routes/index.js +++ b/tests/fixtures/fastboot-app/app/routes/index.js @@ -11,4 +11,8 @@ export default class IndexRoute extends Route { // merged by Embroider. So this serves as a reproduction of https://github.com/embroider-build/embroider/issues/160 return this.fastboot.isFastBoot ? this.fastboot.request.host : null; } + + async model() { + await import('v2-example/components/extra-styles.css'); + } } diff --git a/tests/fixtures/fastboot-app/tests/acceptance/basic-test.js b/tests/fixtures/fastboot-app/tests/acceptance/basic-test.js index b3a540323..c3c20beba 100644 --- a/tests/fixtures/fastboot-app/tests/acceptance/basic-test.js +++ b/tests/fixtures/fastboot-app/tests/acceptance/basic-test.js @@ -40,7 +40,7 @@ module('Acceptance | runtime basics', function (hooks) { assert.equal(secondSampleLib(), 'From second-sample-lib'); }); - test('a component from a v2 addon with css', async function (assert) { + test('a component from a v2 addon with eager css', async function (assert) { assert.dom('[data-test-v2-example]').containsText('it worked'); assert.equal( getComputedStyle(document.querySelector('[data-test-v2-example]')).color, @@ -48,4 +48,13 @@ module('Acceptance | runtime basics', function (hooks) { 'style was applied' ); }); + + test('a component from a v2 addon with lazy css', async function (assert) { + assert.dom('[data-test-v2-example]').containsText('it worked'); + assert.equal( + getComputedStyle(document.querySelector('[data-test-v2-example]')).backgroundColor, + 'rgb(0, 0, 255)', + 'style was applied' + ); + }); }); diff --git a/tests/scenarios/fastboot-app-test.ts b/tests/scenarios/fastboot-app-test.ts index fbe19951a..9fb45b539 100644 --- a/tests/scenarios/fastboot-app-test.ts +++ b/tests/scenarios/fastboot-app-test.ts @@ -51,10 +51,15 @@ appScenarios setComponentTemplate(TEMPLATE, ExampleComponent); `, 'v2-example-component.css': ` - [data-test-v2-example] { + [data-test-v2-example], .eager-styles-marker { color: green; } `, + 'extra-styles.css': ` + [data-test-v2-example], .lazy-styles-marker { + background-color: blue; + } + `, }, }); project.addDependency(v2Example); @@ -129,7 +134,17 @@ appScenarios test('eager CSS from a v2 addon is present', async function (assert) { for (let link of [...doc.querySelectorAll('link[rel="stylesheet"]')]) { let src = await fb.fetchAsset(link.getAttribute('href')!); - if (/\[data-test-v2-example\]/.test(src)) { + if (/eager-styles-marker/.test(src)) { + assert.ok(true, 'found expected style'); + return; + } + } + assert.ok(false, 'did not find expected style'); + }); + test('lazy CSS from a v2 addon is present', async function (assert) { + for (let link of [...doc.querySelectorAll('link[rel="stylesheet"]')]) { + let src = await fb.fetchAsset(link.getAttribute('href')!); + if (/lazy-styles-marker/.test(src)) { assert.ok(true, 'found expected style'); return; } From 25983acfd2f0eee1277a33e9d46b8d5c2430ec40 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Thu, 3 Mar 2022 23:57:20 -0500 Subject: [PATCH 6/8] fixing lazy css This achieves correctness by avoiding use of the MiniCssExtractPlugin runtime inside fastboot. Right now It works by making all the lazy CSS eager. This is strictly better than being broken. Further work can follow to make the lazy chunks actually lazy in fastboot, such that we detect during rendering which ones were wanted and only insert link tags for those ones. --- packages/core/src/html-entrypoint.ts | 22 ++++++++++++++++++---- packages/webpack/src/ember-webpack.ts | 15 ++++++++++++--- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/packages/core/src/html-entrypoint.ts b/packages/core/src/html-entrypoint.ts index 52ecccf65..a66a921e3 100644 --- a/packages/core/src/html-entrypoint.ts +++ b/packages/core/src/html-entrypoint.ts @@ -93,9 +93,11 @@ export class HTMLEntrypoint { if (supportsFastboot && placeholder.isScript()) { // if there is any fastboot involved, we will emit the lazy bundles // right before our first script. + let lazyMatch = stats.lazyBundles.get(src); if (lazyMatch && !insertedLazy.has(src)) { - insertLazyBundles(lazyMatch, placeholder, this.publicAssetURL); + insertLazyJavascript(lazyMatch, placeholder, this.publicAssetURL); + insertLazyStyles(lazyMatch, placeholder, this.publicAssetURL); insertedLazy.add(src); } } @@ -156,9 +158,9 @@ function isAbsoluteURL(url: string) { return /^(?:[a-z]+:)?\/\//i.test(url); } -// we (somewhat arbitrarily) decide to put the lazy bundles before the very -// first