feat(remix): Add Vite support. #495
Conversation
|
There was a problem hiding this comment.
This looks good, thank you Onur!
Just have a couple of small comments.
Also one question: I saw that the "remix reveal" command adds some stuff to the server entry file around handling bot vs human requests. Is this what's present if there was no entry file at all (aka the sensible defaults they talk about) or is this something in addition? I just want to make sure that people do not confuse this with Sentry-relevant code.
src/remix/sdk-setup.ts
Outdated
| 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, | ||
| }); | ||
| } |
There was a problem hiding this comment.
l: Can we just directly call configureVitePlugin? Maybe I missed something but looks like we only call this once. WDYT?
There was a problem hiding this comment.
Yes, makes sense 👍 updated -> 177ab2b
src/remix/sdk-setup.ts
Outdated
| childProcess.execSync(REMIX_REVEAL_COMMAND, { | ||
| stdio: 'inherit', |
There was a problem hiding this comment.
l: Can we pipe the output from the command and wrap it in a clack call? Currently this somewhat breaks the UI:
There was a problem hiding this comment.
Updated 👍 -> a184b9f
src/remix/sdk-setup.ts
Outdated
| childProcess.execSync(REMIX_REVEAL_COMMAND, { | ||
| stdio: 'inherit', |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Ohh right, I missed this, thanks!
As far as I know, those are also a part of their sensible defaults. I have seen them on default Remix application templates too. |
Resolves: #487
Adds Vite sourcemaps upload support to Remix wizard.
Also:
npx run revealwhenentry.client/entry.serverfiles do not exist in the project.project.nameinstead ofproject.slug) while populating build upload script with theprojectargument.