Skip to content

Copy templates into CLI dist folder#326

Merged
jplhomer merged 3 commits intov0.x-2022-10from
jl-tmp-include-templates-in-cli-dist
Dec 14, 2022
Merged

Copy templates into CLI dist folder#326
jplhomer merged 3 commits intov0.x-2022-10from
jl-tmp-include-templates-in-cli-dist

Conversation

@jplhomer
Copy link
Copy Markdown
Contributor

@jplhomer jplhomer commented Dec 13, 2022

Our H2 CLI is kinda tricky to use today. It's an oclif plugin which is meant to be executed by npx @shopify/hydrogen the official CLI, which requires you to install it as a plugin (and then remember to uninstall it for localdev).

It's also super clunky trying to init a template. Because our repo is not open-source, it requires a GitHub token argument, and then you have to hunt down that token.

Instead, as a temporary workaround, I've done the following:

  • Temporarily build templates into cli dist folder
  • Create temporary executable to init app using a dist template

After publishing, this means creating a new H2 app will be as easy as:

# Use default `demo-store` template
npx @shopify/cli-h2-test

# Use a different template
npx @shopify/cli-h2-test /path/to/npx/cli/dist/templates/hello-world

This also makes it trivial to share our progress with external parties without granting them access to our GitHub repos.

We can throw all of this code away once we're open source 👍

@github-actions

This comment has been minimized.

@jplhomer jplhomer changed the title jl tmp include templates in cli dist Copy templates into CLI dist folder Dec 13, 2022
Copy link
Copy Markdown
Contributor

@frandiox frandiox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Terrific idea, Josh! 🎸

Comment on lines +56 to +57
mv templates/demo-store-ts packages/cli/dist/templates/demo-store
mv templates/hello-world-ts packages/cli/dist/templates/hello-world
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only moving the TS versions because the CLI can already generate JS versions on the fly using Remix, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. We only really need to generate the JS versions for use in Oxygen Admin or StackBlitz

Comment on lines +18 to +19
const defaultTemplate = new URL('./dist/templates/demo-store', import.meta.url)
.pathname;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL using URL this way for local paths 💯

@frandiox frandiox requested a review from cartogram December 14, 2022 03:02
@frandiox
Copy link
Copy Markdown
Contributor

@jplhomer Random thought: perhaps can keep this binary executable in the long run instead of having it as temporary?
npx @shopify/cli-hydrogen init would always have value because you don't need to install the main CLI and the plugin beforehand just to create the project, right? Otherwise we need to create a new package for this same purpose.

@@ -1,17 +1,20 @@
import {resolve} from 'path';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need shebang here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, as I believe calling a bin file from NPM will likely call it as node <FILE>. Gonna be a little bit of trial and error here, though.

@jplhomer jplhomer merged commit 781bafc into v0.x-2022-10 Dec 14, 2022
@jplhomer jplhomer deleted the jl-tmp-include-templates-in-cli-dist branch December 14, 2022 14:22
@github-actions github-actions Bot mentioned this pull request Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants