Repository navigation
Conversation
Two small things the service account bug bash turned up in the store commands' output. `store list` piped into a file or a CI log came out squeezed: Ink has no terminal width to go on when stdout isn't a terminal, so it laid the table out in 80 columns, cut the "Type" header to "Typ" and wrapped every cell onto a second line. `renderTable` now gives the table the width it needs in that case; a real terminal keeps its own width and wraps as before. `store info`'s not-found advice told a service account to check it was "signed in to an account with access to the store". A token isn't signed in, and the account's store access is set in the admin, so with a service account token the advice now says what that account can see: the stores it created, or any store in an organization where it has Dev store management access. Co-authored-by: Eddie Gao <eddie.gao@shopify.com>
The listBulkOperations snapshot had captured the table as Ink squeezed it into 80 columns when the test environment has no terminal width: headers cut to "I" and "COMPLET", cells wrapped onto extra lines. renderTable now gives a table the width it needs when stdout isn't a terminal, so the snapshot shows each row on one line, which is what the change is for. Co-authored-by: Eddie Gao <eddie.gao@shopify.com>
Differences in type declarationsWe 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:
New type declarationsWe found no new type declarations in this PR Existing type declarationspackages/cli-kit/dist/private/node/ui/components/Table/Table.d.ts@@ -11,5 +11,13 @@ export interface TableProps<T extends ScalarDict> {
rows: T[];
columns: TableColumn<T>;
}
+/**
+ * The width the table needs to show every cell on one line: the widest value or header of each column, plus the
+ * gap between columns.
+ *
+ * @param props - The table's rows and columns.
+ * @returns The width in characters.
+ */
+export declare function tableWidth<T extends ScalarDict>(props: TableProps<T>): number;
declare function Table<T extends ScalarDict>({ rows, columns: columnsConfiguration }: TableProps<T>): React.JSX.Element;
export { Table };
\ No newline at end of file
|
| * @returns The width in characters. | ||
| */ | ||
| export function tableWidth<T extends ScalarDict>(props: TableProps<T>): number { | ||
| const columns = tableColumns(props) |
There was a problem hiding this comment.
AI-assisted review
Nonblocking: Wide-character store names still get squeezed here. In a local run of the production renderer with stdout piped, a name of '漢'.repeat(32) wrapped the row and clipped neighboring cells—for example, Plus became Plu.
The width calculation uses .length, which counts each 漢 as one, while Ink measures it as two terminal columns. Could we use display width consistently in both the table sizing and Row padding, with a CJK regression test?
This also happens on the base revision, but it leaves part of the display problem this PR addresses unresolved.
WHY are these changes introduced?
Two small things the service accounts bug bash turned up in the store commands' output (P3):
shopify store listpiped into a file or a CI log came out squeezed to 80 columns: the "Type" header read "Typ" and every cell wrapped onto a second line.store info's not-found advice told a service account to check it was "signed in to an account". A token isn't signed in.WHAT is this pull request doing?
renderTable(cli-kit): when stdout isn't a terminal, Ink has no width to go on and lays the table out in 80 columns. The table now gets the width it needs in that case, computed by a newtableWidthhelper extracted from the Table component. A real terminal keeps its own width and wraps as before; banners are untouched.fetchDestinationsContext(store): with a service account token the advice now says what that account can see: the stores it created, or any store in an organization where it has Dev store management access. The login wording stays for people.How to test your changes?
Verified against production (org 299069):
shopify store list | catwith a dev-store token prints one line per row with "Type" whole;shopify store info --store <missing>with a service account token prints the new advice.Unit tests:
ui.test.ts(a table wider than 80 columns stays on one line per row when stdout has no width; an 80-column terminal still wraps),destinations.test.ts(both wordings).tscand eslint pass on cli-kit and store.Measuring impact
Checklist
@shopify/cli-kit,@shopify/cli: patch)