Generate Route CLI Command #365

cartogram · 2023-01-12T17:39:58Z

TLDR: This PR adds a new command to the CLI package that generates a templated routes modules.

To test this PR run npx shopify hydrogen generate route collection from inside the hello-world template. collection can be replaced with any of the following:

cart
product
collection
collections
policies
policy
robots
sitemap

We will add more routes (and refine these existing ones) in follow up PRs. These exist in a new skeleton template inside of this repo. You can run this template locally for convenience when developing or refining the generated routes.

When building and developing the CLI these files are copied to the cli's dist folder so that they are locally available when running the command.

An optional --adaptor flag exists to support projects that are not using the @shopify/remix-oxygen adaptor (the default).

Running the generate route command will extract the route template and put it in the user's local project. It will also format and compile it if they are using JavaScript and respects a local jsconfig and prettierconfig if either exists.

We also prompt if the file already exists (allowing users to overwrite if desired) and this can be bypassed with a --force flag.

caution-tape-bot · 2023-01-12T17:40:12Z

We noticed that this PR either modifies or introduces usage of the dangerouslySetInnerHTML attribute, which can cause cross-site scripting (XSS) vulnerabilities when user controlled values are passed in.
We recommend reviewing your code to ensure that this is what you intended to use and that there is not a safe alternative available.

Docs are available here.

If unavoidable, we reccomend using an HTML sanitizer like DOMPurify to sanitize content before
rendering it as HTML.

If you have any questions or are unsure about how to move forward with this, ping #help-appsec and we would
be happy to help you out! cc: @Shopify/xss-extermination-squad

View the source of this rule in Services DB

github-actions · 2023-01-12T17:40:16Z

We detected some changes in packages/*/package.json or packages/*/src, and there are no updates in the .changeset.
If the changes are user-facing and should cause a version bump, run npm run changeset add to track your changes and include them in the next release CHANGELOG.
If you are making simple updates to examples or documentation, you do not need to add a changeset.

frandiox

Nice, thanks for all the extra stuff added!

In the demo, there was some concern about adding templates/skeleton. How about we move it to generators/skeleton instead? We just need to change a few paths and add it to turbo.json.

packages/cli/src/commands/hydrogen/generate/route.ts

templates/hello-world/package.json

cartogram · 2023-01-17T13:41:18Z

I added a few additional templates (for login, register and page), but would love some additional help getting these all together. @DavidWittness is this something you have interest in helping with?

gfscott

Worked like a charm for the most part!

gfscott · 2023-01-17T20:52:38Z

packages/cli/src/commands/hydrogen/generate/route.ts

+  cart: '/cart',
+  product: '/products/$productHandle',
+  collection: '/collections/$collectionHandle',
+  collections: '/collections/index',


I tried these all out, only collections gave me any trouble:

Oh that makes sense cause there is no collections page 🤦‍♂️ will remove this from the options for now (adding templates will be a whole other thing).

gfscott · 2023-01-17T20:58:48Z

packages/cli/src/commands/hydrogen/generate/route.ts

+  page: '/page/$pageHandle',
+  cart: '/cart',
+  product: '/products/$productHandle',
+  collection: '/collections/$collectionHandle',
+  collections: '/collections/index',
+  policies: '/policies/index',
+  policy: '/policies/$policyHandle',
+  robots: '/[robots.txt]',
+  sitemap: '/[sitemap.xml]',
+  account: ['/account/login', '/account/register'],


Let's consider how and when we'd bundle these together, like you have with the account route.

For instance, you've implemented both collection and collections, to scaffold the $handle pages and the index, respectively. What are the advantages and tradeoffs of doing both at once? To me, it makes sense to just do them both in one go. If they don't want the index page it's easy to delete. Same with page(s), product(s), and polic(y/ies).

Whatever we do, let's make sure that we're consistent with how singular vs. plural option names behave!

Yeah I'm okay with doing both at once and being consistent. It removes some potential confusion or mis-typing between collection and collections.

The logic was added for the account option, so this is more of a product and UX question. @benjaminsehl, @DavidWittness, or @mynameisadamf, let me know any of you guys disagree, otherwise going to move ahead with @gfscott's suggestion.

gfscott · 2023-01-17T21:01:04Z

packages/cli/src/commands/hydrogen/generate/route.ts

+      name: 'resource',
+      description: `The resource to generate a route for.`,
+      required: true,
+      options: RESOURCES,


I think I understand why call it a resource here, but wondering if there's a more specific term that would make sense.

Yeah, maybe route?

Route aligns much better with Hydrogen. I built a prototype for apps yesterday and chose route as well.

gfscott · 2023-01-17T21:03:09Z

packages/cli/src/commands/hydrogen/generate/route.ts

+  }
+
+  // We format the template content with Prettier.
+  // TODO use @shopify/cli-kit's format function once it supports TypeScript


Yes, would love to get to this. The built-in next-steps and links features in cli-kit would be a good chance to point people to the API resources that are most relevant to each route.

@gfscott this comment is specifically about cli-kits file module which has a file.format method that I added ages ago, but needs a little fix before we can use it here. We can still access all of the goodies for outputting nice success messages though, and I'm hoping @mynameisadamf can help with this. If you have something specific/ or an example I can work from, happy to start there.

juanpprieto

Tried tophating but getting 🤷🏼

  [MODULE_NOT_FOUND] require failed to load                                   │
│  /Users/juanp.prieto/code-work/2023/h2/node_modules/@shopify/cli-hydrogen/d  │
│  ist/commands/hydrogen/generate/route

cartogram · 2023-01-18T00:34:24Z

@juanpprieto you aren't the first to report issues with npx 😞 Try running npm run g from the hello-world template instead. For Example: npm run g -- route account and let me know if that works.

juanpprieto · 2023-01-18T00:35:06Z

Update: If you have issues using npx try running npm run g from the hello-world template instead. For Example: npm run g -- route account

akshually I needed to do npm run build at the root.

pepicrft · 2023-01-18T06:45:13Z

packages/cli/package.json

+    "fs-extra": "^10.1.0",
+    "prettier": "^2.8.2",
+    "typescript": "^4.8.3"


In the packages in the CLI we are more strict about the version ranges because breaking changes shipped as minor versions is more usual in Javascript than in other languages where it's easier for developers to catch them. I'd make the call here based on how much you trust that's not going to happen for these dependencies.

pepicrft · 2023-01-18T06:46:46Z

packages/cli/src/commands/hydrogen/generate/route.ts

+
+const RESOURCES = Object.keys(ROUTE_MAP);
+
+export default class GenerateRoute extends Command {


I'd recommend doing a bit of separation of concerns here and extract the business logic from the command. The motivations are two:

Make the logic more reusable

Being able to test it outside of the context of OCLIF.

It's in our roadmap to iterate on the @shopify/cli-kit's APIs to be more conventional about this.

pepicrft · 2023-01-18T06:47:27Z

packages/cli/src/commands/hydrogen/generate/route.ts

+export default class GenerateRoute extends Command {
+  static flags = {
+    path: commonFlags.path,
+    adapter: Flags.string({


I wish we had the ESLint rule here (it's coming), but I'd recommend having an environment variable for each of the flags.

pepicrft · 2023-01-18T06:48:04Z

packages/cli/src/commands/hydrogen/generate/route.ts

+    {
+      name: 'resource',
+      description: `The resource to generate a route for.`,
+      required: true,


Same thing here regarding environment variables

pepicrft · 2023-01-18T06:50:18Z

packages/cli/src/commands/hydrogen/generate/route.ts

+    const isTypescript = await file.exists(
+      path.join(directory, 'tsconfig.json'),
+    );


I'd recommend to have an in-memory representation of a Hydrogen project and have validators that run against it.

const hydrogenProject = await loadProject({from: directory}) await validateIsConfiguredForTypescript(hydrogenProject) // or await validate(hydrogenProject)

pepicrft · 2023-01-18T06:51:10Z

packages/cli/src/commands/hydrogen/generate/route.ts

+    for (const item of [
+      ...(Array.isArray(resourcePath) ? resourcePath : [resourcePath]),
+    ]) {
+      runGenerate(item, {


Who awaits this?

pepicrft · 2023-01-18T06:52:20Z

packages/cli/src/commands/hydrogen/generate/route.ts

+  },
+) {
+  const extension = typescript ? '.tsx' : '.jsx';
+  const distPath = new URL('../../../', import.meta.url).pathname;


const distDirectory = hydrogenProject.directories.dist

pepicrft · 2023-01-18T06:52:50Z

packages/cli/src/commands/hydrogen/generate/route.ts

+) {
+  const extension = typescript ? '.tsx' : '.jsx';
+  const distPath = new URL('../../../', import.meta.url).pathname;
+  const templatePath = path.join(distPath, 'templates', `${resource}.tsx`);


const templatesDirectory = hydrogenProject.directories.dist.templates

pepicrft · 2023-01-18T06:55:54Z

packages/cli/src/commands/hydrogen/generate/route.ts

+      {
+        type: 'select',
+        name: 'value',
+        message: `The file ${path.relative(


We are relativizing twice. You can do path.relativize:

path.relativize(destinationPath)

It'll relativize from the working directory or the root depending on the resulting ../../../../ prefix.

pepicrft · 2023-01-18T06:57:43Z

packages/cli/src/commands/hydrogen/generate/route.ts

+  // to JavaScript. We try to read the project's jsconfig.json, but if it
+  // doesn't exist, we use a default configuration.
+  if (!typescript) {
+    const config = (await file.exists(path.join(directory, 'jsconfig.json')))


A function that generates routes knowing how to set up a project for Typescript doesn't feel right:

const remixProject = loadRemixProject({fromDirectory: directory}) if (!remixProject.isSetupForTypescript()) { remixProject.setupForTypescript() }

pepicrft · 2023-01-18T06:58:37Z

packages/cli/src/commands/hydrogen/generate/route.ts

+          lib: ['DOM', 'DOM.Iterable', 'ES2022'],
+          isolatedModules: true,
+          esModuleInterop: true,
+          resolveJsonModule: true,
+          target: 'ES2022',
+          strict: true,
+          allowJs: true,
+          forceConsistentCasingInFileNames: true,
+          skipLibCheck: true,


Is this content disconnected from the version of Remix the project is using? If not, I'd recommend obtaining it from the project and having a map between Remix version and TSConfig

Co-authored-by: Fran Dios <frankdiox@gmail.com>

cartogram requested a review from pepicrft January 12, 2023 17:40

cartogram requested a review from a team January 12, 2023 17:40

github-actions bot deployed to preview January 12, 2023 22:08 View deployment

frandiox reviewed Jan 13, 2023

View reviewed changes

frandiox requested a review from a team January 13, 2023 10:05

github-actions bot deployed to preview January 13, 2023 10:58 View deployment

github-actions bot deployed to preview January 17, 2023 12:57 View deployment

gfscott reviewed Jan 17, 2023

View reviewed changes

juanpprieto reviewed Jan 18, 2023

View reviewed changes

pepicrft reviewed Jan 18, 2023

View reviewed changes

pepicrft approved these changes Jan 18, 2023

View reviewed changes

cartogram mentioned this pull request Jan 20, 2023

Set TypeScript parser explicitly when formatting .ts and .tsx files Shopify/cli#1142

Merged

6 tasks

Matt Seccafien added 3 commits January 20, 2023 05:23

Initial route generate command

f5f1421

Add more templates

a837fa2

Delete cart.tsx

9fdf496

Matt Seccafien and others added 8 commits January 20, 2023 05:23

Add --force flag, make directory and fix prettier

fc6aa3b

Remove non-existent templates from route map

3a15ea8

Update packages/cli/src/commands/hydrogen/generate/route.ts

614c624

Co-authored-by: Fran Dios <frankdiox@gmail.com>

Add page template

e30efa8

Add CLI log for template generation

c8f0f0d

Support multiple files for a given resource (ie: account)

005fe48

reviewer feedback

c86ab60

Update package-lock.json

0880ece

cartogram force-pushed the generate branch from 5feba75 to 0880ece Compare January 20, 2023 15:23

github-actions bot deployed to preview January 20, 2023 15:25 View deployment

cartogram merged commit 1dd01f7 into v0.x-2022-10 Jan 20, 2023

cartogram deleted the generate branch January 20, 2023 15:38

cartogram mentioned this pull request Jan 25, 2023

Add a skeleton template #338

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generate Route CLI Command #365

Generate Route CLI Command #365

cartogram commented Jan 12, 2023

caution-tape-bot bot commented Jan 12, 2023

github-actions bot commented Jan 12, 2023

frandiox left a comment

cartogram commented Jan 17, 2023

gfscott left a comment

gfscott Jan 17, 2023

cartogram Jan 17, 2023

gfscott Jan 17, 2023

cartogram Jan 17, 2023 •

edited

Loading

gfscott Jan 17, 2023

cartogram Jan 17, 2023

pepicrft Jan 18, 2023

gfscott Jan 17, 2023

cartogram Jan 17, 2023

juanpprieto left a comment •

edited

Loading

cartogram commented Jan 18, 2023 •

edited

Loading

juanpprieto commented Jan 18, 2023

pepicrft Jan 18, 2023

pepicrft Jan 18, 2023

pepicrft Jan 18, 2023

pepicrft Jan 18, 2023

pepicrft Jan 18, 2023

pepicrft Jan 18, 2023

pepicrft Jan 18, 2023

pepicrft Jan 18, 2023

pepicrft Jan 18, 2023

pepicrft Jan 18, 2023

pepicrft Jan 18, 2023


		const RESOURCES = Object.keys(ROUTE_MAP);

		export default class GenerateRoute extends Command {

Generate Route CLI Command #365

Generate Route CLI Command #365

Conversation

cartogram commented Jan 12, 2023

caution-tape-bot bot commented Jan 12, 2023

github-actions bot commented Jan 12, 2023

frandiox left a comment

Choose a reason for hiding this comment

cartogram commented Jan 17, 2023

gfscott 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

cartogram Jan 17, 2023 • edited Loading

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

juanpprieto left a comment • edited Loading

Choose a reason for hiding this comment

cartogram commented Jan 18, 2023 • edited Loading

juanpprieto commented Jan 18, 2023

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

cartogram Jan 17, 2023 •

edited

Loading

juanpprieto left a comment •

edited

Loading

cartogram commented Jan 18, 2023 •

edited

Loading