Remote Templates #329

lukeed · 2017-08-21T03:55:22Z

What

Allows cloning of Preact CLI compatible templates from remote repositories.

This adds a separation-of-concerns layer, relinquishing all of the what (aka, the contents) to third parties. In effect, this allows the community to share & reuse templates or recipes that the Preact CLI team is not experienced with and/or not willing to officially support.

Our community is very bright & inventive, so allowing them to create solutions to their own needs is a win for everybody! 🎉

As a result, Preact CLI will focus on two items:

Scaffolding the initial project files
Enhancing its commands for:
a) optimal PWA-centric builds
b) user-friendly development process

Why

Open the doors to community-driven templates
Reusable custom templates
Separation of concerns
Offline support

How

Use an official template:

$ preact create default my-app --yarn --git
#=> 1. Downloads 'preactjs-templates/default' locally
#=> 2. Extracts 'default/master.tar.gz' to '$CWD/my-app'
#=> 3. Installs with Yarn 
#=> 4. Inits a Git repo

Use a user template (tagged version!):

$ preact create lukeed/preact-starter#v1.1.0 my-app --name FooBar
#=> 1. Checks local cache for v1.1.0 of 'lukeed/preact-starter'
#=> 2. Downloads & Caches tag archive if not already cached
#=> 3. Extracts to '$CWD/my-app'
#=> 4. Changes 'name' within resolved 'package.json'
#=> 5. Changes 'name' within resolved 'manifest.json'
#=> 6. Installs with NPM

TODOs

Prompt the user when insufficient data is passed to preact create
Alias preact init to preact create
Read & Respond to "extraction hooks"
Injecting values based on custom prompts
Allow optional, custom prompts on a per-template basis
Template authors may require specific values or have a choice list for certain items

Still a WIP. I'd like to continue only once all TODO items are complete.

Closes #99, #326.

- declare `log-symbols` as a dependency - it is already a direct dependency of `ora`

- with nice symbol & formatting

- uses *sync versions, but oh well

- will need to verify a destination before extracting/downloading - will prompt (TODO) if incomplete - will abort if unsafe

- use more partial import statements

- uses which.sync 🙊 - update function calls - remove `async` from `pkgScripts`

- instead of checking 5x per `setup` function - return `async` function directly from `initialize`

- template concern

- only `default` & `full` for now

- PRs are still built

- somehow `recursive-copy` was picked up?

- WTF HAPPENING

lukeed · 2017-08-23T20:30:34Z

Is there any chance a Travis caching is making this error? 🤔

I've run this a few times locally (using the same Node 8.x version) and the build tests work fine.

reznord · 2017-08-25T17:57:59Z

src/commands/create.js

 			}
+		} else {
+			// TODO: interactive


@developit Should we really do this? This make it more complex for us to handle.

Let's save this discussion for a future PR. I want to break it up instead of making this (even more) massive.

reznord · 2017-08-25T18:07:21Z

tests/images/create.js

+exports.default = [
+	'.gitignore',
+	'package.json',
+	'README.md',


Even when we use node-glob, it sorts in the alphabetical order AFAIK. This might be an issue.

It does. The above is alpha-sorted 😄

and it always fails in my machine 😛

- no need anymore! - templates responsible own dependecies - and style-* flags removed

- OCD strikes again

- tell user to run `npm install` first

lukeed · 2017-08-26T18:05:07Z

Fixed the tests! I made the (silly) mistake of not passing the Travis flags to my local tests 🤦‍♂️ . Once I did, was clear that the failures did relate to the dependency-loader... The signature on setup.js@install changed, and it was "silently" failing (got lost in stack output).

However, in fixing it, I realized we don't even need the dependency-installer anymore!

Reason being, templates are responsible for themselves. The rest of this PR assumes that, with the _tiny exception of adding package scripts if none were defined. So, in keeping the philosophy consistent, we shouldn't be installing critical build dependencies on behalf of the user.

Aside: This requires the 4 test "subjects" to install deps... we can have Travis use yarn soon.

It's no longer our job in remote-template world. 🎉

More PRs to come, which will address all tasks & gotchas I've mentioned.

reznord

Apart from these comments, its gold. 👍

reznord · 2017-08-30T13:30:56Z

src/commands/create.js

-	root: 'examples/root',
-	simple: 'examples/simple'
+	full: 'preactjs-templates/default',
+	default: 'preactjs-templates/default',


why both full & default? Since they do the same thing.

Instead we can just add default & simple from templates.

We should add simple. @developit requested in Slack that we make default an alias of full

Was just thinking "principle of least surprise" here, this makes it work quite similarly to how it does in 1.0.

reznord · 2017-08-30T13:45:04Z

src/commands/create.js

+			filter(path) {
+				// TODO: remove `/build/` ??
+				// TODO: read & respond to meta/hooks
+				return path.includes('/template/') && !path.includes('/build/');


If we don't find templates/ folder, we can throw an error saying that you need a specific folder structure or sth like that.

Good point. Let's do that in another PR.

return error(`No \`template\` directory found within ${ repo }!`);

Could even detect template/ and if it doesn't exist, assume the template is at the root. I'm fine with either, and a subsequent PR seems fine.

I think we need to force a template structure. When things become too flexible they become less clear and harder to debug.

reznord · 2017-08-30T13:56:34Z

src/lib/setup.js

-		dev: 'preact watch',
-		test: 'eslint src && preact test'
+		start: `if-env NODE_ENV=production && ${cmd} run -s serve || ${cmd} run -s watch`,
+		watch: 'preact watch'


we should probably leave it to dev, since most users expect that 😄

Lol okay. I thought this would make more sense since the command name is watch.

I waffled on this incessantly when doing the original scripts. FWIW CRA does neither, they actually use npm start for the dev server (that's why I added the env bit there).

I don't have a really compelling reason to go one way or the other here. It might be annoying for people to upgrade and find the command they know has a new name, but then it's only in the templated projects - anything created with 1.x will work just fine.

Yeah, however, it is a major release, so this would be the time to change it, if any.

reznord · 2017-08-30T14:20:50Z

tests/build.test.js

+
+// TODO
+// const ours = ['empty', 'full', 'simple', 'root'];
+const ours = ['default', 'full'];


It should be default & simple here too! (coz we can run tests on those templates)

Same as above ^^

reznord · 2017-08-30T14:28:15Z

tests/images/create.js

+exports.default = [
+	'.gitignore',
+	'package.json',
+	'README.md',


and it always fails in my machine 😛

reznord

👍 LGTM!

ping @developit for further review on this 😄

developit · 2017-09-01T17:55:13Z

src/commands/create.js

-	root: 'examples/root',
-	simple: 'examples/simple'
+	full: 'preactjs-templates/default',
+	default: 'preactjs-templates/default',


Was just thinking "principle of least surprise" here, this makes it work quite similarly to how it does in 1.0.

developit · 2017-09-01T17:57:08Z

src/commands/create.js

-			process.exit(1);
-		}
+		let repo = TEMPLATES[argv.template] || argv.template;
+


Would it be reasonable here to do something like: "if repo has no /, assume preactjs-templates/${repo}"? That way any template we add to the preactjs-templates org becomes a default option.

Sure. Or just include new official templates (should not be often!) as patch releases.

developit · 2017-09-01T17:58:43Z

src/commands/create.js

+			filter(path) {
+				// TODO: remove `/build/` ??
+				// TODO: read & respond to meta/hooks
+				return path.includes('/template/') && !path.includes('/build/');


Could even detect template/ and if it doesn't exist, assume the template is at the root. I'm fine with either, and a subsequent PR seems fine.

developit · 2017-09-01T17:59:56Z

src/commands/create.js

+		if (pkgFile) {
+			pkgData = JSON.parse(await fs.readFile(pkgFile));
+			// Write default "scripts" if none found
+			pkgData.scripts = pkgData.scripts || (await addScripts(pkgData, target, isYarn));


Maybe instead of making scripts a wholesale override, we could merge our scripts into the template's?

pkgData.scripts = { ...(pkgData.scripts || {}), ...(await addScripts(pkgData, target, isYarn)) };

we could merge our scripts into the template's?

I don't think there is a necessary for that since templates will have scripts defined in the package.json file (includes our scripts too)

Just sucks if the cli wants to change script down the line 😋

developit · 2017-09-01T18:05:21Z

src/lib/setup.js

-		dev: 'preact watch',
-		test: 'eslint src && preact test'
+		start: `if-env NODE_ENV=production && ${cmd} run -s serve || ${cmd} run -s watch`,
+		watch: 'preact watch'


I waffled on this incessantly when doing the original scripts. FWIW CRA does neither, they actually use npm start for the dev server (that's why I added the env bit there).

I don't have a really compelling reason to go one way or the other here. It might be annoying for people to upgrade and find the command they know has a new name, but then it's only in the templated projects - anything created with 1.x will work just fine.

developit · 2017-09-01T18:07:14Z

src/lib/webpack/webpack-base-config.js

@@ -141,13 +134,6 @@ export default (env) => {
 									loader: 'sass-loader',
 									options: { sourceMap: true }
 								}
-							},
-							{


What the best way for templates to add these?

preact.config.js is the only way the user can tweak the webpack config in CLI as of now.

Template authors have to define their own dependencies, as a rule. As of now, the template can also include a preact.config.js, but in the near future, I'll add a meta.js file (from root) that will have postinstall hooks and behaviors. Hat tip to Vue for that.

- print `info` message about the assumption

lukeed added 30 commits August 19, 2017 13:55

install gittar

71a6b6c

remove flags: css,sass,less,dest,type

d4ff638

purge *css flag-related stuffs

6c3c3aa

extract yarn-vs-npm logic before printing

b7f4cdc

replace ansi-codes with chalk.green

427b22e

extract pkgFile path

8ab7688

use argv.name within pkg & manifest (Closes #326)

6b878d4

- declare `log-symbols` as a dependency - it is already a direct dependency of `ora`

extract an error helper

86a4f3b

- with nice symbol & formatting

uninstall recursive-copy

1ee0dfc

extract isDir helper

5e4fdb0

- uses *sync versions, but oh well

replace copy with gittar.extract - with filter

a23765e

move target + exists + force logic first;

172ff54

- will need to verify a destination before extracting/downloading - will prompt (TODO) if incomplete - will abort if unsafe

use argv.dest for final instructions

f04532f

fix typo

4dc79c0

export setup functions directly

8daf606

remove debug-type stdout from initGit

eccca4f

move common helpers to util file;

15f1b6b

- use more partial import statements

move commandExists —> util.hasCommand;

45b28c2

- uses which.sync 🙊 - update function calls - remove `async` from `pkgScripts`

check if yarn bin exists on flag parsing;

ce8d316

- instead of checking 5x per `setup` function - return `async` function directly from `initialize`

consolidate setup.pkgScripts function

60e4830

do not write a gitignore file

34379f5

- template concern

minor cleanup, warn if no git binary found

8dc0583

oops, fix regex tester

da46a8b

don’t assume exit within util.error

1e18c1a

use warn within watch command

c443554

use util.isDir within serve command

39091a7

remove init command publicly

8af1c30

Merge master into templates

dfec01b

alias “official” preact-cli templates by name

c3430c7

- only `default` & `full` for now

simplify install — install template deps

54b93ae

lukeed added 3 commits August 23, 2017 11:13

[Travis] only build master branch

fb8f273

- PRs are still built

remove commands/init file

b3380e0

- somehow `recursive-copy` was picked up?

add console statement for debug

f8f7ec4

- WTF HAPPENING

reznord reviewed Aug 25, 2017

View reviewed changes

lukeed added 5 commits August 26, 2017 10:09

remove dependency-install-loader

1880a38

- no need anymore! - templates responsible own dependecies - and style-* flags removed

update scripts path

bc7af44

- OCD strikes again

exit early if run build w/o installing deps;

0ed79d1

- tell user to run `npm install` first

always install deps for build subjects

faa8cad

remove SKIP_INSTALL global for Travis (temp)

741774e

lukeed requested review from developit and reznord August 26, 2017 18:05

This was referenced Aug 26, 2017

All routes are chunked, causing hydration of prerendered pages to flicker #281

Closed

Add support for typescript #11

Closed

reznord reviewed Aug 30, 2017

View reviewed changes

reznord approved these changes Sep 1, 2017

View reviewed changes

developit reviewed Sep 1, 2017

View reviewed changes

lukeed added 2 commits September 2, 2017 19:50

assume any template without / is official lookup

7c1ad41

- print `info` message about the assumption

update tests, remove full alias

e7ee7f7

This was referenced Sep 4, 2017

Run TravisCLI with Yarn #350

Closed

Error if no template dir found #351

Closed

Prompt user if insufficient args #352

Closed

Inject Values during Template extraction #353

Closed

Template Extraction Hooks #354

Closed

developit merged commit f1891fe into master Sep 4, 2017

lukeed deleted the templates branch September 4, 2017 15:33

snyk-bot mentioned this pull request Apr 16, 2021

[Snyk] Security upgrade copy-webpack-plugin from 4.6.0 to 5.0.0 chagretes/preact-cli#15

Open

snyk-bot mentioned this pull request Aug 17, 2021

[Snyk] Security upgrade copy-webpack-plugin from 4.6.0 to 5.0.0 chagretes/preact-cli#18

Open

Remote Templates #329

Remote Templates #329

Conversation

lukeed commented Aug 21, 2017

What

Why

How

TODOs

lukeed commented Aug 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukeed commented Aug 26, 2017

reznord left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reznord left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment