Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

General fixes #1128

Merged
merged 10 commits into from
Jul 21, 2023
6 changes: 6 additions & 0 deletions .changeset/five-bees-itch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@shopify/hydrogen-react': patch
'@shopify/hydrogen': patch
---

Throw error when `storeDomain` is not passed to `createStorefrontClient`.
15 changes: 13 additions & 2 deletions packages/cli/src/commands/hydrogen/list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,17 @@ import {
outputInfo,
outputNewline,
} from '@shopify/cli-kit/node/output';
import {renderInfo} from '@shopify/cli-kit/node/ui';
import {commonFlags} from '../../lib/flags.js';
import {parseGid} from '../../lib/gid.js';
import {
type Deployment,
type HydrogenStorefront,
getStorefrontsWithDeployment,
} from '../../lib/graphql/admin/list-storefronts.js';
import {logMissingStorefronts} from '../../lib/missing-storefronts.js';
import {newHydrogenStorefrontUrl} from '../../lib/admin-urls.js';
import {login} from '../../lib/auth.js';
import {getCliCommand} from '../../lib/shell.js';

export default class List extends Command {
static description =
Expand Down Expand Up @@ -75,7 +77,16 @@ export async function runList({path: root = process.cwd()}: Flags) {
},
);
} else {
logMissingStorefronts(session);
renderInfo({
headline: 'Hydrogen storefronts',
body: 'There are no Hydrogen storefronts on your Shop.',
nextSteps: [
`Ensure you are logged in to the correct shop (currently: ${session.storeFqdn})`,
`Create a new Hydrogen storefront: Run \`${await getCliCommand(
root,
)} link\` or visit ${newHydrogenStorefrontUrl(session)}`,
],
});
}
}

Expand Down
16 changes: 8 additions & 8 deletions packages/cli/src/lib/log.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {resetAllLogs, enhanceH2Logs} from './log.js';
describe('log replacer', () => {
describe('enhanceH2Logs', () => {
const graphiqlUrl = 'http://localhost:3000/graphiql';
const appDirectory = fileURLToPath(import.meta.url);
const rootDirectory = fileURLToPath(import.meta.url);
const outputMock = mockAndCaptureOutput();

beforeEach(() => {
Expand All @@ -21,7 +21,7 @@ describe('log replacer', () => {

describe('enhances h2:info pattern', () => {
it('renders in an info banner', () => {
enhanceH2Logs({graphiqlUrl, appDirectory});
enhanceH2Logs({graphiqlUrl, rootDirectory});

console.warn('[h2:info:storefront.query] Tip');

Expand All @@ -35,7 +35,7 @@ describe('log replacer', () => {

describe('enhances h2:warn pattern', () => {
it('renders in a warning banner', () => {
enhanceH2Logs({graphiqlUrl, appDirectory});
enhanceH2Logs({graphiqlUrl, rootDirectory});

console.warn('[h2:warn:storefront.query] Wrong query 1');

Expand All @@ -47,7 +47,7 @@ describe('log replacer', () => {
});

it('shows links from the last line as a list', () => {
enhanceH2Logs({graphiqlUrl, appDirectory});
enhanceH2Logs({graphiqlUrl, rootDirectory});

console.warn(
'[h2:warn:storefront.query] Wrong query.\nhttps://docs.com/something',
Expand All @@ -62,7 +62,7 @@ describe('log replacer', () => {

describe('enhances h2:error pattern', () => {
it('renders in an error banner', () => {
enhanceH2Logs({graphiqlUrl, appDirectory});
enhanceH2Logs({graphiqlUrl, rootDirectory});

console.error(new Error('[h2:error:storefront.query] Wrong query 2'));

Expand All @@ -75,7 +75,7 @@ describe('log replacer', () => {
});

it('shows a GraphiQL link when the error is related to a GraphQL query', () => {
enhanceH2Logs({graphiqlUrl, appDirectory});
enhanceH2Logs({graphiqlUrl, rootDirectory});

console.error(
new Error('[h2:error:storefront.query] Wrong query 3', {
Expand All @@ -95,7 +95,7 @@ describe('log replacer', () => {
});

it('trims stack traces when the error is related to a GraphQL query', () => {
enhanceH2Logs({graphiqlUrl, appDirectory});
enhanceH2Logs({graphiqlUrl, rootDirectory});

console.error(
new Error('[h2:error:storefront.query] Wrong query 4', {
Expand All @@ -105,7 +105,7 @@ describe('log replacer', () => {

const error = outputMock.error();
const stack = error.split('stack trace:')[1] ?? '';
const shortenedAppDir = appDirectory.split('/cli/').pop()!;
const shortenedAppDir = rootDirectory.split('/cli/').pop()!;
expect(stack).toMatch(shortenedAppDir);
expect(stack).not.toMatch('node_modules');
});
Expand Down
27 changes: 15 additions & 12 deletions packages/cli/src/lib/log.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ export function muteAuthLogs({
*/
export function enhanceH2Logs(options: {
graphiqlUrl: string;
appDirectory: string;
rootDirectory: string;
}) {
injectLogReplacer('warn');
injectLogReplacer('error');
Expand Down Expand Up @@ -228,18 +228,21 @@ export function enhanceH2Logs(options: {
queryName ? ` \`${colors.whiteBright(queryName)}\`` : ''
}, try it in ${outputToken.link(colors.bold('GraphiQL'), link)}.`
.value;
}

// Sanitize stack trace to only show app code
const stackLines = stack?.split('\n') ?? [];
const isAppLine = (line: string) =>
line.includes(options.appDirectory);
const firstAppLineIndex = stackLines.findIndex(isAppLine);
const lastAppLineIndex =
stackLines.length -
[...stackLines]
.reverse() // findLastIndex requires Node 18
.findIndex(isAppLine);

// Sanitize stack trace to only show app code
const stackLines = stack?.split('\n') ?? [];
const isAppLine = (line: string) =>
line.includes(options.rootDirectory) &&
!line.includes('node_modules');
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it not work with just the node_modules check?

Copy link
Contributor Author

@frandiox frandiox Jul 21, 2023

Choose a reason for hiding this comment

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

It... probably does 🤔

Edit: I forgot why I did it but now I remember -- it should work only with the node_modules check in user apps but not in the monorepo where part of the stack comes from packages/hydrogen 😅

const firstAppLineIndex = stackLines.findIndex(isAppLine);
const lastAppLineIndex =
stackLines.length -
[...stackLines]
.reverse() // findLastIndex requires Node 18
.findIndex(isAppLine);

if (firstAppLineIndex > 0 && lastAppLineIndex > firstAppLineIndex) {
stack =
[
stackLines[0], // Error message
Expand Down
17 changes: 0 additions & 17 deletions packages/cli/src/lib/missing-storefronts.ts

This file was deleted.

2 changes: 0 additions & 2 deletions packages/cli/src/lib/shopify-config.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import {resolvePath, dirname} from '@shopify/cli-kit/node/path';
import {readFile, mkdir, fileExists, writeFile} from '@shopify/cli-kit/node/fs';
import {AbortError} from '@shopify/cli-kit/node/error';
import {outputInfo} from '@shopify/cli-kit/node/output';

export const SHOPIFY_DIR = '.shopify';
export const SHOPIFY_DIR_PROJECT = 'project.json';
Expand Down Expand Up @@ -146,7 +145,6 @@ export async function ensureShopifyGitIgnore(root: string): Promise<boolean> {

gitIgnoreContents += `${SHOPIFY_DIR}\r\n`;

outputInfo('Adding .shopify to .gitignore...\n');
await writeFile(gitIgnoreFilePath, gitIgnoreContents);

return true;
Expand Down
16 changes: 0 additions & 16 deletions packages/cli/src/lib/user-errors.ts

This file was deleted.

51 changes: 35 additions & 16 deletions packages/cli/src/virtual-routes/routes/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,26 @@ export default function Index() {
{configDone ? <HydrogenLogoBaseColor /> : <HydrogenLogoBaseBW />}
<h1>Hello, {shopName}</h1>
<p>Welcome to your new custom storefront</p>
{configDone ? null : (
<section className="Banner">
<div>
<IconBanner />
<h2>Configure storefront token</h2>
</div>

<section className="Banner">
<div>
<IconBanner />
<h2>
{configDone
? 'Create your first route'
: 'Configure storefront token'}
</h2>
</div>
{configDone ? (
<p>
You&rsquo;re seeing this because you don&rsquo;t have a home route
in your project yet. <br />
Run <code>h2 generate route home</code> to create your home route.
Learn more about
{` `}
<CreateRoutesLink />
Comment on lines +86 to +91
Copy link
Contributor

Choose a reason for hiding this comment

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

So much better

</p>
) : (
<p>
You&rsquo;re seeing this because you have not yet configured your
storefront token. <br />
Expand All @@ -95,23 +109,28 @@ export default function Index() {
</a>
{` `}
and{` `}
<a
target="_blank"
rel="norefferer noopener"
href="https://shopify.dev/docs/custom-storefronts/hydrogen/building/begin-development#step-4-create-a-route"
>
creating routes
</a>
.
<CreateRoutesLink />.
</p>
</section>
)}
)}
</section>
<ResourcesLinks />
</Layout>
</>
);
}

function CreateRoutesLink() {
return (
<a
target="_blank"
rel="norefferer noopener"
href="https://shopify.dev/docs/custom-storefronts/hydrogen/building/begin-development#step-4-create-a-route"
>
creating routes
</a>
);
}

function ErrorPage() {
return (
<>
Expand Down
11 changes: 9 additions & 2 deletions packages/hydrogen-react/src/storefront-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ export function createStorefrontClient(
contentType,
} = props;

if (!storeDomain) {
throw new Error(
H2_PREFIX_ERROR +
`\`storeDomain\` is required when creating a new Storefront client.\nReceived "${storeDomain}".`,
);
}

if (storefrontApiVersion !== SFAPI_VERSION) {
warnOnce(
`The Storefront API version that you're using is different than the version this build of Hydrogen React is targeting.` +
Expand Down Expand Up @@ -84,7 +91,7 @@ export function createStorefrontClient(
) {
throw new Error(
H2_PREFIX_ERROR +
'You did not pass in a `privateStorefrontToken` while using `getPrivateTokenHeaders()`',
'You did not pass in a `privateStorefrontToken` while using `createStorefrontClient()` or `getPrivateTokenHeaders()`',
);
}

Expand Down Expand Up @@ -120,7 +127,7 @@ export function createStorefrontClient(
) {
throw new Error(
H2_PREFIX_ERROR +
'You did not pass in a `publicStorefrontToken` while using `getPublicTokenHeaders()`',
'You did not pass in a `publicStorefrontToken` while using `createStorefrontClient()` or `getPublicTokenHeaders()`',
);
}

Expand Down
5 changes: 2 additions & 3 deletions templates/hello-world/.env
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# The variables added in this file are only available locally in MiniOxygen
# These variables are only available locally in MiniOxygen

SESSION_SECRET="foobar"
PUBLIC_STOREFRONT_API_TOKEN="3b580e70970c4528da70c98e097c2fa0"
PUBLIC_STORE_DOMAIN="hydrogen-preview.myshopify.com"
PUBLIC_STORE_DOMAIN="mock.shop"