From cd8bfc8a47bec0814c8859359681b75720a2078c Mon Sep 17 00:00:00 2001 From: Marine Dunstetter Date: Fri, 22 Mar 2024 15:13:58 +0100 Subject: [PATCH 1/5] feat(compile route templates): add a component to v2-addon-dev-test to drive development --- tests/scenarios/v2-addon-dev-test.ts | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/scenarios/v2-addon-dev-test.ts b/tests/scenarios/v2-addon-dev-test.ts index c0ad046f8..af5841447 100644 --- a/tests/scenarios/v2-addon-dev-test.ts +++ b/tests/scenarios/v2-addon-dev-test.ts @@ -113,6 +113,7 @@ appScenarios flip `, + 'just-a-template.hbs': `

I am not a component but a template.

`, 'out.hbs': ` {{yield}} `, @@ -124,10 +125,12 @@ appScenarios import { tracked } from '@glimmer/tracking'; import FlipButton from './button'; + import JustATemplate from './just-a-template'; import Out from './out'; export default class ExampleComponent extends Component { Button = FlipButton; + JustATemplate = JustATemplate; Out = Out; @tracked active = false; @@ -137,6 +140,8 @@ appScenarios `, 'index.hbs': `Hello there! + + {{this.active}} @@ -244,6 +249,7 @@ appScenarios './components/demo/button.js': './dist/_app_/components/demo/button.js', './components/single-file-component.js': './dist/_app_/components/single-file-component.js', './components/demo/index.js': './dist/_app_/components/demo/index.js', + './components/demo/just-a-template.js': './dist/_app_/components/demo/just-a-template.js', './components/demo/out.js': './dist/_app_/components/demo/out.js', './components/demo/namespace/namespace-me.js': './dist/_app_/components/demo/namespace/namespace-me.js', }); @@ -251,6 +257,7 @@ appScenarios test('the addon has expected public entrypoints', async function () { expectFile('dist/components/demo/index.js').exists(); + expectFile('dist/components/demo/just-a-template.js').exists(); expectFile('dist/components/demo/out.js').exists(); expectFile('dist/components/demo/namespace-me.js').exists(); expectFile('dist/components/-excluded/never-import-this.js').doesNotExist(); @@ -260,6 +267,9 @@ appScenarios expectFile('dist/_app_/components/demo/index.js').matches( 'export { default } from "v2-addon/components/demo/index"' ); + expectFile('dist/_app_/components/demo/just-a-template.js').matches( + 'export { default } from "v2-addon/components/demo/just-a-template"' + ); expectFile('dist/_app_/components/demo/out.js').matches( 'export { default } from "v2-addon/components/demo/out"' ); @@ -274,6 +284,16 @@ appScenarios /TEMPLATE = precompileTemplate\("Hello there/, 'template is still in hbs format' ); + + expectFile( + 'dist/components/demo/just-a-template.js' + ).equalsCode(`import templateOnly from '@ember/component/template-only'; +import { precompileTemplate } from '@ember/template-compilation'; +import { setComponentTemplate } from '@ember/component'; +var TEMPLATE = precompileTemplate("

I am not a component but a template.

"); +var JustATemplate = setComponentTemplate(TEMPLATE, templateOnly()); +export { JustATemplate as default }; +//# sourceMappingURL=just-a-template.js.map`); }); test('gjs components compiled correctly', async function () { From d0387c2732dc05f20af56628440f6f73132d63f6 Mon Sep 17 00:00:00 2001 From: Marine Dunstetter Date: Tue, 26 Mar 2024 11:01:37 +0100 Subject: [PATCH 2/5] feat(compile route templates): remove the templateOnly import from a specified hbs file --- packages/addon-dev/src/rollup-hbs-plugin.ts | 49 ++++++++++++++++----- packages/addon-dev/src/rollup.ts | 4 +- tests/scenarios/v2-addon-dev-test.ts | 15 +++---- 3 files changed, 45 insertions(+), 23 deletions(-) diff --git a/packages/addon-dev/src/rollup-hbs-plugin.ts b/packages/addon-dev/src/rollup-hbs-plugin.ts index 9b3efb92b..b93b03e0b 100644 --- a/packages/addon-dev/src/rollup-hbs-plugin.ts +++ b/packages/addon-dev/src/rollup-hbs-plugin.ts @@ -2,9 +2,14 @@ import { createFilter } from '@rollup/pluginutils'; import type { Plugin, PluginContext, CustomPluginOptions } from 'rollup'; import { readFileSync } from 'fs'; import { hbsToJS } from '@embroider/core'; +import minimatch from 'minimatch'; import { parse as pathParse } from 'path'; -export default function rollupHbsPlugin(): Plugin { +export default function rollupHbsPlugin({ + templates, +}: { + templates?: string[]; +}): Plugin { return { name: 'rollup-hbs-plugin', async resolveId(source: string, importer: string | undefined, options) { @@ -16,19 +21,26 @@ export default function rollupHbsPlugin(): Plugin { if (resolution) { return resolution; } else { - return maybeSynthesizeComponentJS(this, source, importer, options); + return maybeSynthesizeComponentJS( + this, + source, + importer, + options, + templates + ); } }, load(id: string) { if (hbsFilter(id)) { - let input = readFileSync(id, 'utf8'); - let code = hbsToJS(input); - return { - code, - }; + return getHbsToJSCode(id); } - if (getMeta(this, id)) { + let meta = getMeta(this, id); + if (meta) { + if (meta?.type === 'template-js') { + const hbsFile = id.replace(/\.js$/, '.hbs'); + return getHbsToJSCode(hbsFile); + } return { code: templateOnlyComponent, }; @@ -42,7 +54,7 @@ const templateOnlyComponent = `export default templateOnly();\n`; type Meta = { - type: 'template-only-component-js'; + type: 'template-only-component-js' | 'template-js'; }; function getMeta(context: PluginContext, id: string): Meta | null { @@ -54,6 +66,14 @@ function getMeta(context: PluginContext, id: string): Meta | null { } } +function getHbsToJSCode(file: string): { code: string } { + let input = readFileSync(file, 'utf8'); + let code = hbsToJS(input); + return { + code, + }; +} + function correspondingTemplate(filename: string): string { let { ext } = pathParse(filename); return filename.slice(0, filename.length - ext.length) + '.hbs'; @@ -63,7 +83,8 @@ async function maybeSynthesizeComponentJS( context: PluginContext, source: string, importer: string | undefined, - options: { custom?: CustomPluginOptions; isEntry: boolean } + options: { custom?: CustomPluginOptions; isEntry: boolean }, + templates: string[] | undefined ) { let templateResolution = await context.resolve( correspondingTemplate(source), @@ -76,13 +97,17 @@ async function maybeSynthesizeComponentJS( if (!templateResolution) { return null; } + let type = templates?.some((glob) => minimatch(source, glob)) + ? 'template-js' + : 'template-only-component-js'; // we're trying to resolve a JS module but only the corresponding HBS - // file exists. Synthesize the template-only component JS. + // file exists. Synthesize the JS. The meta states if the hbs corresponds + // to a template-only component or a simple template like a route template. return { id: templateResolution.id.replace(/\.hbs$/, '.js'), meta: { 'rollup-hbs-plugin': { - type: 'template-only-component-js', + type, }, }, }; diff --git a/packages/addon-dev/src/rollup.ts b/packages/addon-dev/src/rollup.ts index f4056128f..debe26491 100644 --- a/packages/addon-dev/src/rollup.ts +++ b/packages/addon-dev/src/rollup.ts @@ -53,8 +53,8 @@ export class Addon { // This wraps standalone .hbs files as Javascript files using inline // templates. This means special resolving rules for .hbs files aren't // required for javascript tooling to understand your package. - hbs() { - return hbs(); + hbs(options = {}) { + return hbs(options); } gjs(options?: { inline_source_map: boolean }) { diff --git a/tests/scenarios/v2-addon-dev-test.ts b/tests/scenarios/v2-addon-dev-test.ts index af5841447..b2a0f1eac 100644 --- a/tests/scenarios/v2-addon-dev-test.ts +++ b/tests/scenarios/v2-addon-dev-test.ts @@ -71,7 +71,9 @@ appScenarios exclude: ['**/-excluded/**/*'], }), - addon.hbs(), + addon.hbs({ + templates: ['**/just-a-template.js'], + }), addon.gjs(), addon.dependencies(), @@ -125,12 +127,10 @@ appScenarios import { tracked } from '@glimmer/tracking'; import FlipButton from './button'; - import JustATemplate from './just-a-template'; import Out from './out'; export default class ExampleComponent extends Component { Button = FlipButton; - JustATemplate = JustATemplate; Out = Out; @tracked active = false; @@ -140,8 +140,6 @@ appScenarios `, 'index.hbs': `Hello there! - - {{this.active}} @@ -287,12 +285,11 @@ appScenarios expectFile( 'dist/components/demo/just-a-template.js' - ).equalsCode(`import templateOnly from '@ember/component/template-only'; -import { precompileTemplate } from '@ember/template-compilation'; + ).equalsCode(`import { precompileTemplate } from '@ember/template-compilation'; import { setComponentTemplate } from '@ember/component'; var TEMPLATE = precompileTemplate("

I am not a component but a template.

"); -var JustATemplate = setComponentTemplate(TEMPLATE, templateOnly()); -export { JustATemplate as default }; +var justATemplate = setComponentTemplate(TEMPLATE, precompileTemplate("

I am not a component but a template.

")); +export { justATemplate as default }; //# sourceMappingURL=just-a-template.js.map`); }); From e870de5b48287b3af27b74bad252549a33bbc5cb Mon Sep 17 00:00:00 2001 From: Marine Dunstetter Date: Tue, 26 Mar 2024 13:48:31 +0100 Subject: [PATCH 3/5] feat(compile route templates): skip template-colocation-plugin for route templates --- packages/shared-internals/package.json | 2 ++ .../src/template-colocation-plugin.ts | 14 ++++++++++++++ pnpm-lock.yaml | 6 ++++++ tests/scenarios/v2-addon-dev-test.ts | 8 ++++---- 4 files changed, 26 insertions(+), 4 deletions(-) diff --git a/packages/shared-internals/package.json b/packages/shared-internals/package.json index 24b503ff1..3c8509318 100644 --- a/packages/shared-internals/package.json +++ b/packages/shared-internals/package.json @@ -36,6 +36,7 @@ "typescript-memoize": "^1.0.1", "fs-extra": "^9.1.0", "lodash": "^4.17.21", + "minimatch": "^3.0.4", "semver": "^7.3.5" }, "devDependencies": { @@ -47,6 +48,7 @@ "@types/fs-extra": "^9.0.12", "@types/lodash": "^4.14.170", "@types/js-string-escape": "^1.0.0", + "@types/minimatch": "^3.0.4", "@types/semver": "^7.3.6", "@types/tmp": "^0.1.0", "fixturify": "^2.1.1", diff --git a/packages/shared-internals/src/template-colocation-plugin.ts b/packages/shared-internals/src/template-colocation-plugin.ts index 52f830b1e..4fcbaab0d 100644 --- a/packages/shared-internals/src/template-colocation-plugin.ts +++ b/packages/shared-internals/src/template-colocation-plugin.ts @@ -6,6 +6,7 @@ import { dirname } from 'path'; import { explicitRelative, PackageCache } from '.'; import { ImportUtil } from 'babel-import-util'; import makeDebug from 'debug'; +import minimatch from 'minimatch'; import { cleanUrl } from './paths'; const debug = makeDebug('embroider:template-colocation-plugin'); @@ -36,6 +37,14 @@ export interface Options { // This option is used by Embroider itself to help with v1 addon // compatibility, other users should probably not use it. templateExtensions?: string[]; + + // Default to [] + // + // Skip the plugin for files that match the specified globs. + // + // This option is used to prevent the plugin to transform the + // compiled output of hbs files that are not colocated components. + exclude?: string[]; } interface State { @@ -66,6 +75,11 @@ export default function main(babel: typeof Babel) { } } + if (state.opts.exclude?.some(glob => minimatch(filename, glob))) { + debug('not handling colocation for %s', filename); + return; + } + debug('handling colocation for %s', filename); let extensions = state.opts.templateExtensions ?? ['.hbs']; for (let ext of extensions) { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 28ff7a638..8286cd35a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -665,6 +665,9 @@ importers: lodash: specifier: ^4.17.21 version: 4.17.21 + minimatch: + specifier: ^3.0.4 + version: 3.1.2 resolve-package-path: specifier: ^4.0.1 version: 4.0.3 @@ -696,6 +699,9 @@ importers: '@types/lodash': specifier: ^4.14.170 version: 4.14.202 + '@types/minimatch': + specifier: ^3.0.4 + version: 3.0.5 '@types/semver': specifier: ^7.3.6 version: 7.5.8 diff --git a/tests/scenarios/v2-addon-dev-test.ts b/tests/scenarios/v2-addon-dev-test.ts index b2a0f1eac..a68673073 100644 --- a/tests/scenarios/v2-addon-dev-test.ts +++ b/tests/scenarios/v2-addon-dev-test.ts @@ -29,7 +29,9 @@ appScenarios 'babel.config.json': ` { "plugins": [ - "@embroider/addon-dev/template-colocation-plugin", + ["@embroider/addon-dev/template-colocation-plugin", { + exclude: ['**/just-a-template.js'], + }], "@babel/plugin-transform-class-static-block", ["babel-plugin-ember-template-compilation", { targetFormat: 'hbs', @@ -286,9 +288,7 @@ appScenarios expectFile( 'dist/components/demo/just-a-template.js' ).equalsCode(`import { precompileTemplate } from '@ember/template-compilation'; -import { setComponentTemplate } from '@ember/component'; -var TEMPLATE = precompileTemplate("

I am not a component but a template.

"); -var justATemplate = setComponentTemplate(TEMPLATE, precompileTemplate("

I am not a component but a template.

")); +var justATemplate = precompileTemplate("

I am not a component but a template.

"); export { justATemplate as default }; //# sourceMappingURL=just-a-template.js.map`); }); From 566925f26f920d18fd2773093809e990ceac7099 Mon Sep 17 00:00:00 2001 From: Marine Dunstetter Date: Fri, 29 Mar 2024 12:42:38 +0100 Subject: [PATCH 4/5] feat(compile route templates): improve API so users can specify their hbs files instead of js that don't exist in their codebase --- packages/addon-dev/src/rollup-hbs-plugin.ts | 31 +++++++------------ packages/shared-internals/src/index.ts | 2 +- packages/shared-internals/src/paths.ts | 9 +++++- .../src/template-colocation-plugin.ts | 4 +-- tests/scenarios/v2-addon-dev-test.ts | 4 +-- 5 files changed, 24 insertions(+), 26 deletions(-) diff --git a/packages/addon-dev/src/rollup-hbs-plugin.ts b/packages/addon-dev/src/rollup-hbs-plugin.ts index b93b03e0b..db35a35fd 100644 --- a/packages/addon-dev/src/rollup-hbs-plugin.ts +++ b/packages/addon-dev/src/rollup-hbs-plugin.ts @@ -1,14 +1,13 @@ import { createFilter } from '@rollup/pluginutils'; import type { Plugin, PluginContext, CustomPluginOptions } from 'rollup'; import { readFileSync } from 'fs'; -import { hbsToJS } from '@embroider/core'; +import { correspondingTemplate, hbsToJS } from '@embroider/core'; import minimatch from 'minimatch'; -import { parse as pathParse } from 'path'; export default function rollupHbsPlugin({ - templates, + excludeColocation, }: { - templates?: string[]; + excludeColocation?: string[]; }): Plugin { return { name: 'rollup-hbs-plugin', @@ -26,7 +25,7 @@ export default function rollupHbsPlugin({ source, importer, options, - templates + excludeColocation ); } }, @@ -74,30 +73,22 @@ function getHbsToJSCode(file: string): { code: string } { }; } -function correspondingTemplate(filename: string): string { - let { ext } = pathParse(filename); - return filename.slice(0, filename.length - ext.length) + '.hbs'; -} - async function maybeSynthesizeComponentJS( context: PluginContext, source: string, importer: string | undefined, options: { custom?: CustomPluginOptions; isEntry: boolean }, - templates: string[] | undefined + excludeColocation: string[] | undefined ) { - let templateResolution = await context.resolve( - correspondingTemplate(source), - importer, - { - skipSelf: true, - ...options, - } - ); + let hbsFilename = correspondingTemplate(source); + let templateResolution = await context.resolve(hbsFilename, importer, { + skipSelf: true, + ...options, + }); if (!templateResolution) { return null; } - let type = templates?.some((glob) => minimatch(source, glob)) + let type = excludeColocation?.some((glob) => minimatch(hbsFilename, glob)) ? 'template-js' : 'template-only-component-js'; // we're trying to resolve a JS module but only the corresponding HBS diff --git a/packages/shared-internals/src/index.ts b/packages/shared-internals/src/index.ts index cbf236beb..2cb7f5b42 100644 --- a/packages/shared-internals/src/index.ts +++ b/packages/shared-internals/src/index.ts @@ -1,5 +1,5 @@ export { AppMeta, AddonMeta, PackageInfo } from './metadata'; -export { explicitRelative, extensionsPattern, unrelativize, cleanUrl } from './paths'; +export { explicitRelative, extensionsPattern, unrelativize, cleanUrl, correspondingTemplate } from './paths'; export { getOrCreate } from './get-or-create'; export { default as Package, V2AddonPackage as AddonPackage, V2AppPackage as AppPackage, V2Package } from './package'; export { default as PackageCache } from './package-cache'; diff --git a/packages/shared-internals/src/paths.ts b/packages/shared-internals/src/paths.ts index a1e0ed123..5d5ea6dbe 100644 --- a/packages/shared-internals/src/paths.ts +++ b/packages/shared-internals/src/paths.ts @@ -1,4 +1,4 @@ -import { relative, isAbsolute, dirname, join, basename, resolve, sep } from 'path'; +import { relative, isAbsolute, dirname, join, basename, resolve, sep, parse as pathParse } from 'path'; import type Package from './package'; // by "explicit", I mean that we want "./local/thing" instead of "local/thing" @@ -49,3 +49,10 @@ const postfixRE = /[?#].*$/s; export function cleanUrl(url: string): string { return url.replace(postfixRE, ''); } + +// given a filename, returns it with the hbs extension +// for instance, passing filename.js returns filename.hbs +export function correspondingTemplate(filename: string): string { + let { ext } = pathParse(filename); + return filename.slice(0, filename.length - ext.length) + '.hbs'; +} diff --git a/packages/shared-internals/src/template-colocation-plugin.ts b/packages/shared-internals/src/template-colocation-plugin.ts index 4fcbaab0d..8004dbadb 100644 --- a/packages/shared-internals/src/template-colocation-plugin.ts +++ b/packages/shared-internals/src/template-colocation-plugin.ts @@ -7,7 +7,7 @@ import { explicitRelative, PackageCache } from '.'; import { ImportUtil } from 'babel-import-util'; import makeDebug from 'debug'; import minimatch from 'minimatch'; -import { cleanUrl } from './paths'; +import { cleanUrl, correspondingTemplate } from './paths'; const debug = makeDebug('embroider:template-colocation-plugin'); @@ -75,7 +75,7 @@ export default function main(babel: typeof Babel) { } } - if (state.opts.exclude?.some(glob => minimatch(filename, glob))) { + if (state.opts.exclude?.some(glob => minimatch(correspondingTemplate(filename), glob))) { debug('not handling colocation for %s', filename); return; } diff --git a/tests/scenarios/v2-addon-dev-test.ts b/tests/scenarios/v2-addon-dev-test.ts index a68673073..b08435f93 100644 --- a/tests/scenarios/v2-addon-dev-test.ts +++ b/tests/scenarios/v2-addon-dev-test.ts @@ -30,7 +30,7 @@ appScenarios { "plugins": [ ["@embroider/addon-dev/template-colocation-plugin", { - exclude: ['**/just-a-template.js'], + exclude: ['**/just-a-template.hbs'], }], "@babel/plugin-transform-class-static-block", ["babel-plugin-ember-template-compilation", { @@ -74,7 +74,7 @@ appScenarios }), addon.hbs({ - templates: ['**/just-a-template.js'], + excludeColocation: ['**/just-a-template.hbs'], }), addon.gjs(), addon.dependencies(), From f0cd0291bd4ab7a479413c615356652de5c6af9c Mon Sep 17 00:00:00 2001 From: Marine Dunstetter Date: Wed, 10 Apr 2024 16:04:36 +0200 Subject: [PATCH 5/5] docs(compile route templates): add a section to the faq about providing route templates with a v1 addon --- docs/v2-faq.md | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/docs/v2-faq.md b/docs/v2-faq.md index 542c5c3b9..8a7f9b075 100644 --- a/docs/v2-faq.md +++ b/docs/v2-faq.md @@ -17,6 +17,7 @@ * [Why do v2 addons need a build step?](#why-do-v2-addons-need-a-build-step) * [How can I integrate with the app's build?](#how-can-i-integrate-with-the-apps-build) * [How can I define the public exports of my addon?](#how-can-i-define-the-public-exports-of-my-addon) + * [How can I provide route templates with my v2 addon?](#how-can-i-provide-route-templates-with-my-v2-addon) @@ -195,3 +196,29 @@ Additionally, there is a feature supported in node.js and modern bundlers to def When using `package.json#exports` make sure that: - the `addon.publicEntrypoints(...)` plugin in `rollup.config.mjs` includes _at least_ whatever is defined in `package.json#exports` - the modules that `addon.appReexports(...)` exposes must have overlap with the `package.json#exports` so that the app-tree merging may import from the addon + +### How can I provide route templates with my v2 addon? + +During a v2 addon build step, standalone `.hbs` are considered template-only components by default. + +If you want your v2 addon to provide a route template, the best way to proceed is to make it a `.gjs` file using [ember-route-template](https://github.com/discourse/ember-route-template). Similarly, if you want to migrate to v2 a classic addon that used to provide `.hbs` route templates, you should refactor the `.hbs` to `.gjs` files to complete the migration. + +If for whatever reason the `.gjs` approach cannot be used, it's still possible to have your v2 addon providing the route templates as `.hbs`, but it requires extra configuration. During the build step, Rollup and Babel work together to transform all standalone `.hbs` into template-only components. Therefore, you need to tell both Rollup and Babel to _not_ compile a given list of `.hbs` files this way. + +Let's assume your addon has a `templates/` folder that contains all your route templates. The files in `templates/` should be compiled as simple templates (not template-only components). + +In the `rollup.config.mjs`, pass a list of glob patterns in the `excludeColocation` option of the function `addon.hbs`: + +```js +addon.hbs({ excludeColocation: ['templates/**/*'] }), +``` + +In the `babel.config.json`, pass the same list of glob patterns in the `exclude` option of the `template-colocation-plugin`: + +``` +"plugins": [ + ["@embroider/addon-dev/template-colocation-plugin", { + exclude: ['templates/**/*'] + }], +], +``` \ No newline at end of file