Rework app info to use components #5170

amcaplan · 2025-01-08T11:46:19Z

WHY are these changes introduced?

Replaces messy string generation code with orderly use of UI-kit.
Hides the web section for extension-only apps.

WHAT is this pull request doing?

Instead of the mess of string generation, we reorganize the code to generate headers and data tables, then do the actual rendering in UI kit.

To make this work nicely, I added a new TabularData concept to UI kit, which does seem useful in general. It even has a parameter to dim the leftmost column, which I think creates a pleasant effect.

App with many extensions, before:

And after:

Extension-only app, before:

And after:

App with many errors, before:

And after:

How to test your changes?

Run shopify app info on various apps.

Measuring impact

How do we know this change was effective? Please choose one:

n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix

Checklist

I've considered possible cross-platform impacts (Mac, Linux, Windows)
I've considered possible documentation changes

github-actions · 2025-01-09T12:21:44Z

Coverage report

St.^❔	Category	Percentage	Covered / Total
🟡	Statements	75.04% (-0.14% 🔻)	8867/11817
🟡	Branches	70.12% (-0.2% 🔻)	4327/6171
🟡	Functions	74.97% (-0.12% 🔻)	2321/3096
🟡	Lines	75.59% (-0.14% 🔻)	8383/11090

Show new covered files 🐣

St.^❔	File	Statements	Branches	Functions	Lines
🔴	`...` / TabularData.tsx	12.5%	0%	0%	12.5%

Show files with reduced coverage 🔻

St.^❔	File	Statements	Branches	Functions	Lines
🟢	`...` / info.ts	81.44% (-6.74% 🔻)	60.94% (-2.52% 🔻)	90.32%	83.15% (-7.85% 🔻)
🟢	`...` / app-event-watcher.ts	93.83% (-1.23% 🔻)	86.49% (-2.7% 🔻)	90.48%	98.61%
🟢	`...` / Alert.tsx	100%	92% (-3.24% 🔻)	100%	100%
🟢	`...` / FatalError.tsx	100%	86.11% (-1.39% 🔻)	100%	100%

Test suite run success

2003 tests passing in 904 suites.

Report generated by 🧪jest coverage report action from 7df0f33

graygilmore

I won't speak to the introduction of a new component but this is such a massive improvement for apps – thank you!

graygilmore · 2025-01-09T16:41:58Z

packages/app/src/cli/services/info.ts

+      this.tableSection(
+        'Current app configuration',
+        [
+          ['Configuration file', {filePath: basename(this.app.configuration.path) || configurationFileNames.app}],


Just flagging that the filePath token gets rendered pretty differently depending on your terminal colors. I don't mind that it's different but I wanted to point it out!

This is very surprising! It's supposed to be italicized, I wonder what your settings are such that colors are inverted instead.

isaacroldan · 2025-01-13T12:20:35Z

packages/app/src/cli/services/info.ts

 import {CLI_KIT_VERSION} from '@shopify/cli-kit/common/version'

+type CustomSection = Exclude<Parameters<typeof renderInfo>[0]['customSections'], undefined>[number]


Can you add a comment to clarify what this type represents? Asking because hovering on it in Cursor just says type CustomSection = CustomSection 😅

I realized we do already export the AlertCustomSection type so I'm just going to use that.

isaacroldan · 2025-01-13T12:26:19Z

packages/app/src/cli/services/info.ts

-      ['     config file', relativePath(extension.directory, extension.configurationPath)],
+    const details: InlineToken[][] = [
+      [`📂 ${extension.handle || NOT_LOADED_TEXT}`, {filePath: relativePath(this.app.directory, extension.directory)}],
+      ['     config file', {filePath: relativePath(extension.directory, extension.configurationPath)}],


this is probably unnecessary, since the config file is always shopify.extension.toml.

Should we remove it?

I think it is still valuable for consistency and for new users who are still learning how apps work.

isaacroldan · 2025-01-13T12:29:56Z

packages/app/src/cli/services/info.ts

+      {
+        body: [
+          '💡 To change these, run',
+          {command: formatPackageManagerCommand(this.app.packageManager, 'dev', '--reset')},


since we are modifying info, it's probably better to recommend running app config link.

Also, formatPackageManagerCommand expects a full global command as default, so this should be:

{command: formatPackageManagerCommand(this.app.packageManager, 'shopify app config link')},

Happy to make the change and I think it's an improvement. But I have some thoughts about making this better in general, since now it won't really account for changing the dev store. Basically this was set up at a time when the CLI worked differently, and it needs a refresh.

Agree we need to rework that section 👌

1. Add a default relative path (otherwise relativePath returns a blank string) 2. Move roles to a separate line item

We removed the upgrade reminders in #4310 but we missed deleting the tests!

…rtners

github-actions · 2025-01-13T16:27:00Z

Differences in type declarations

We detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:

Some seemingly private modules might be re-exported through public modules.
If the branch is behind main you might see odd diffs, rebase main into this branch.

New type declarations

packages/cli-kit/dist/private/node/ui/components/TabularData.d.ts

import { InlineToken } from './TokenizedText.js';
import { FunctionComponent } from 'react';
export interface TabularDataProps {
    tabularData: InlineToken[][];
    firstColumnSubdued?: boolean;
}
declare const TabularData: FunctionComponent<TabularDataProps>;
export { TabularData };

Existing type declarations

packages/cli-kit/dist/private/node/ui/components/Alert.d.ts

@@ -1,9 +1,10 @@
 import { BannerType } from './Banner.js';
 import { BoldToken, InlineToken, LinkToken, TokenItem } from './TokenizedText.js';
+import { TabularDataProps } from './TabularData.js';
 import { FunctionComponent } from 'react';
 export interface CustomSection {
     title?: string;
-    body: TokenItem;
+    body: TabularDataProps | TokenItem;
 }
 export interface AlertProps {
     type: Exclude<BannerType, 'external_error'>;

amcaplan force-pushed the app-info-rework branch 5 times, most recently from e4cc408 to 8144601 Compare January 9, 2025 12:17

amcaplan force-pushed the app-info-rework branch 2 times, most recently from 3bf58a5 to 2d18f38 Compare January 9, 2025 14:27

amcaplan marked this pull request as ready for review January 9, 2025 15:32

amcaplan requested a review from a team as a code owner January 9, 2025 15:32

This comment has been minimized.

Sign in to view

amcaplan requested a review from a team as a code owner January 9, 2025 15:51

graygilmore approved these changes Jan 9, 2025

View reviewed changes

amcaplan force-pushed the app-info-rework branch from 3970ccc to 413f784 Compare January 9, 2025 19:23

isaacroldan reviewed Jan 13, 2025

View reviewed changes

amcaplan requested a review from isaacroldan January 13, 2025 13:06

amcaplan force-pushed the app-info-rework branch from 413f784 to 362151e Compare January 13, 2025 13:08

isaacroldan approved these changes Jan 13, 2025

View reviewed changes

amcaplan added this pull request to the merge queue Jan 13, 2025

github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Jan 13, 2025

amcaplan added 8 commits January 13, 2025 18:20

Rework app info to use components

b02d1f9

Improve handling of error messages

17d42a2

Improve display of web section

0db0229

1. Add a default relative path (otherwise relativePath returns a blank string) 2. Move roles to a separate line item

Update tests to match new rendering system

2c643e3

Remove no-longer-relevant tests

6ce820a

We removed the upgrade reminders in #4310 but we missed deleting the tests!

Lint fixes

3ce2ba2

Refer to User instead of Partners account because we may not be on Pa…

e8ee754

…rtners

Add changesets

bb3b100

amcaplan added 4 commits January 13, 2025 18:22

Render app name as userInput token

b19fce5

Add example of tabularData to kitchen sink

261ee48

Use exported AlertCustomSection type

67b19b7

Suggest more updated command

7429878

amcaplan force-pushed the app-info-rework branch from 362151e to 7429878 Compare January 13, 2025 16:23

Lint fix

7df0f33

amcaplan added this pull request to the merge queue Jan 13, 2025

Merged via the queue into main with commit 6391f7b Jan 13, 2025
27 checks passed

amcaplan deleted the app-info-rework branch January 13, 2025 18:03

shopify-shipit bot temporarily deployed to nightly January 14, 2025 10:58 Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework app info to use components #5170

Rework app info to use components #5170

amcaplan commented Jan 8, 2025 •

edited

Loading

github-actions bot commented Jan 9, 2025 •

edited

Loading

This comment has been minimized.

graygilmore left a comment

graygilmore Jan 9, 2025

amcaplan Jan 9, 2025

isaacroldan Jan 13, 2025 •

edited

Loading

amcaplan Jan 13, 2025

isaacroldan Jan 13, 2025

amcaplan Jan 13, 2025

isaacroldan Jan 13, 2025

amcaplan Jan 13, 2025

isaacroldan Jan 13, 2025

github-actions bot commented Jan 13, 2025

		import {CLI_KIT_VERSION} from '@shopify/cli-kit/common/version'

		type CustomSection = Exclude<Parameters<typeof renderInfo>[0]['customSections'], undefined>[number]

Rework app info to use components #5170

Rework app info to use components #5170

Conversation

amcaplan commented Jan 8, 2025 • edited Loading

WHY are these changes introduced?

WHAT is this pull request doing?

How to test your changes?

Measuring impact

Checklist

github-actions bot commented Jan 9, 2025 • edited Loading

Coverage report

Test suite run success

This comment has been minimized.

graygilmore left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

isaacroldan Jan 13, 2025 • 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

Choose a reason for hiding this comment

github-actions bot commented Jan 13, 2025

Differences in type declarations

New type declarations

Existing type declarations

amcaplan commented Jan 8, 2025 •

edited

Loading

github-actions bot commented Jan 9, 2025 •

edited

Loading

isaacroldan Jan 13, 2025 •

edited

Loading