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

feat(remix): Add Vite support. #495

Merged
merged 6 commits into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 50 additions & 15 deletions src/remix/remix-wizard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,14 @@ import {
instrumentRootRoute,
isRemixV2,
loadRemixConfig,
runRemixReveal,
modifyViteConfig,
} from './sdk-setup';
import { debug } from '../utils/debug';
import { traceStep, withTelemetry } from '../telemetry';
import { isHydrogenApp } from './utils';
import { DEFAULT_URL } from '../../lib/Constants';
import { findFile } from '../utils/ast-utils';

export async function runRemixWizard(options: WizardOptions): Promise<void> {
return withTelemetry(
Expand Down Expand Up @@ -55,7 +58,7 @@ async function runRemixWizardWithTelemetry(
// We expect `@remix-run/dev` to be installed for every Remix project
await ensurePackageIsInstalled(packageJson, '@remix-run/dev', 'Remix');

const { selectedProject, authToken, sentryUrl } =
const { selectedProject, authToken, sentryUrl, selfHosted } =
await getOrAskForProjectData(options, 'javascript-remix');

await installPackage({
Expand All @@ -67,24 +70,46 @@ async function runRemixWizardWithTelemetry(

const isTS = isUsingTypeScript();
const isV2 = isRemixV2(remixConfig, packageJson);
const viteConfig = findFile('vite.config');

await addSentryCliConfig({ authToken }, rcCliSetupConfig);

await traceStep('Update build script for sourcemap uploads', async () => {
try {
await updateBuildScript({
org: selectedProject.organization.slug,
project: selectedProject.name,
url: sentryUrl === DEFAULT_URL ? undefined : sentryUrl,
isHydrogen: isHydrogenApp(packageJson),
});
} catch (e) {
clack.log
.warn(`Could not update build script to generate and upload sourcemaps.
if (viteConfig) {
await traceStep(
'Update vite configuration for sourcemap uploads',
async () => {
try {
await modifyViteConfig(
selectedProject,
sentryUrl,
authToken,
selfHosted,
);
} catch (e) {
clack.log
.warn(`Could not update vite configuration to generate and upload sourcemaps.
Please update your vite configuration manually using instructions from https://docs.sentry.io/platforms/javascript/guides/remix/sourcemaps/`);
debug(e);
}
},
);
} else {
await traceStep('Update build script for sourcemap uploads', async () => {
try {
await updateBuildScript({
org: selectedProject.organization.slug,
project: selectedProject.slug,
url: sentryUrl === DEFAULT_URL ? undefined : sentryUrl,
isHydrogen: isHydrogenApp(packageJson),
});
} catch (e) {
clack.log
.warn(`Could not update build script to generate and upload sourcemaps.
Please update your build script manually using instructions from https://docs.sentry.io/platforms/javascript/guides/remix/sourcemaps/`);
debug(e);
}
});
debug(e);
}
});
}

await traceStep('Instrument root route', async () => {
try {
Expand All @@ -96,6 +121,16 @@ async function runRemixWizardWithTelemetry(
}
});

traceStep('Reveal missing entry files', () => {
try {
runRemixReveal(isTS);
} catch (e) {
clack.log.warn(`Could not run 'npx remix reveal'.
Please create your entry files manually`);
debug(e);
}
});

await traceStep('Initialize Sentry on client entry', async () => {
try {
await initializeSentryOnEntryClient(dsn, isTS);
Expand Down
52 changes: 52 additions & 0 deletions src/remix/sdk-setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import type { ProxifiedModule } from 'magicast';
import * as fs from 'fs';
import * as path from 'path';
import * as url from 'url';
import * as childProcess from 'child_process';

// @ts-expect-error - clack is ESM and TS complains about that. It works though
import clack from '@clack/prompts';
Expand All @@ -22,6 +23,7 @@ import { instrumentRootRouteV1 } from './codemods/root-v1';
import { instrumentRootRouteV2 } from './codemods/root-v2';
import { instrumentHandleError } from './codemods/handle-error';
import { getPackageDotJson } from '../utils/clack-utils';
import { configureVitePlugin } from '../sourcemaps/tools/vite';

export type PartialRemixConfig = {
unstable_dev?: boolean;
Expand All @@ -36,6 +38,52 @@ export type PartialRemixConfig = {
};

const REMIX_CONFIG_FILE = 'remix.config.js';
const REMIX_REVEAL_COMMAND = 'npx remix reveal';

export function runRemixReveal(isTS: boolean): void {
// Check if entry files already exist
const clientEntryFilename = `entry.client.${isTS ? 'tsx' : 'jsx'}`;
const serverEntryFilename = `entry.server.${isTS ? 'tsx' : 'jsx'}`;

const clientEntryPath = path.join(process.cwd(), 'app', clientEntryFilename);
const serverEntryPath = path.join(process.cwd(), 'app', serverEntryFilename);

if (fs.existsSync(clientEntryPath) && fs.existsSync(serverEntryPath)) {
clack.log.info(
`Found entry files ${chalk.cyan(clientEntryFilename)} and ${chalk.cyan(
serverEntryFilename,
)}.`,
);
} else {
clack.log.info(
`Couldn't find entry files in your project. Trying to run ${chalk.cyan(
REMIX_REVEAL_COMMAND,
)}...`,
);

childProcess.execSync(REMIX_REVEAL_COMMAND, {
stdio: 'inherit',
Copy link
Member

Choose a reason for hiding this comment

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

l: Can we pipe the output from the command and wrap it in a clack call? Currently this somewhat breaks the UI:
image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated 👍 -> a184b9f

Copy link
Member

Choose a reason for hiding this comment

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

I don't know how remix reveal works but should we check for the return value or try/catch it? Just to ensure we don't crash but e.g. link to the manual setup if something goes wrong.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is handled on the remix-wizard.ts. This will throw on any non-zero exit code, which will be caught on the upper level.

Copy link
Member

Choose a reason for hiding this comment

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

Ohh right, I missed this, thanks!

});
}
}

export async function modifyViteConfig(
selectedProject: {
organization: { slug: string };
slug: string;
},
sentryUrl: string,
authToken: string,
selfHosted: boolean,
): Promise<void> {
await configureVitePlugin({
orgSlug: selectedProject.organization.slug,
projectSlug: selectedProject.slug,
url: sentryUrl,
selfHosted,
authToken,
});
}
Copy link
Member

Choose a reason for hiding this comment

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

l: Can we just directly call configureVitePlugin? Maybe I missed something but looks like we only call this once. WDYT?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, makes sense 👍 updated -> 177ab2b


function insertClientInitCall(
dsn: string,
Expand Down Expand Up @@ -192,6 +240,10 @@ export async function updateBuildScript(args: {
buildCommand,
instrumentedBuildCommand,
);
} else {
throw new Error(
"`build` script doesn't contain a known build command. Please update it manually.",
);
}

await fs.promises.writeFile(
Expand Down
Loading