Uh oh!
There was an error while loading. Please reload this page.
feat(nuxt): Add server config to root folder - #13583
Conversation
size-limit report 📦
|
6cd63cb to
ea95e37Comparenandi95
commented
Sep 6, 2024
This indeed would be helpful as I have a project which a merge of a laravel a nuxt project. Nuxt project is in the folder This indeed |
s1gr1d
commented
Sep 6, 2024
@nandi95 If I understand you correctly, you are defining a curstom |
nandi95
commented
Sep 6, 2024
I only meant to say that I define a |
| if (moduleOptions.debug) { | ||
| // eslint-disable-next-line no-console | ||
| console.log( | ||
| '[Sentry] Using your `sentry.server.config` file for the server-side Sentry configuration. In case you have a `public/instrument.server` file, it will be ignored.', |
There was a problem hiding this comment.
(...) it will be ignored
This reads a bit confusing, which of the two files are being ignored?
I think we could check here for the existence of the file and log explicitly if that file is getting ignored, wdyt?
Also, we are ignoring the public file here but users will still potentially --import it. This log makes it sound like it's completely ignored.
There was a problem hiding this comment.
I think this log should be split up.
1st part (public): can we log a warning if we detect the file in public instead of the first part of this message? Otherwise, I don't think we need to bother users with it, right?
To the second part: Can we instead print the exact import command? Something like
"Make sure to add --import ./.output/sentry.server.config.mjs to your NODE_OPTIONS env variable"
| if (moduleOptions.debug) { | ||
| // eslint-disable-next-line no-console | ||
| console.log( | ||
| '[Sentry] Successfully added the content of the `sentry.server.config` file as `instrument-sentry.mjs` to the `.output/server` directory', |
There was a problem hiding this comment.
l: Maybe we can use destination here?
Uh oh!
There was an error while loading. Please reload this page.
Lms24
left a comment
There was a problem hiding this comment.
Nice work! Other than the console log, this looks good to me!
We can follow up with the e2e tests after this is merged, given we hit some road blocks there
| it('should return the server file with .ts extension if it exists', () => { | ||
| vi.spyOn(fs, 'existsSync').mockImplementation(filePath => { | ||
| return !(filePath instanceof URL) && filePath.includes('sentry.server.config.ts'); | ||
| }); | ||
| const result = findDefaultSdkInitFile('server'); | ||
| expect(result).toBe('sentry.server.config.ts'); | ||
| }); | ||
| it('should return the client file with .mjs extension if it exists', () => { | ||
| vi.spyOn(fs, 'existsSync').mockImplementation(filePath => { | ||
| return !(filePath instanceof URL) && filePath.includes('sentry.client.config.mjs'); | ||
| }); | ||
| const result = findDefaultSdkInitFile('client'); | ||
| expect(result).toBe('sentry.client.config.mjs'); | ||
| }); |
There was a problem hiding this comment.
l: For some test extra credit (feel free to disregard) - we could use an it.each to test against all file types we search for.
wanderowski
commented
Sep 10, 2024
Hello! Thank you for the work you have done! Could you please clarify, should we somehow modify the command for running the project on server in package.json? I see that now we can put sentry.server.config.ts in the root folder, but did not find any instructions on how to modify starting command. Previously, it was required to paste this: If i put my config file in the root folder and deleted instrument.server.mjs from public folder, should I modify these commands as well? |
@wanderowski This PR is not released yet and more elaborate docs on this are still coming for the official docs page. The file can be added like this: "start": "node --import ./.output/server/sentry.server.config.mjs .output/server/index.mjs",As it only affects the server, it can only be added to the However, it could be that your environment variables from the |
wanderowski
commented
Sep 10, 2024
@s1gr1d Thanks! |
Makes it possible to include a
sentry.server.config.tsfile in the root folder alongsidesentry.client.config.ts. Currently, it has to be added in thepublicfolder which is not 100% ideal.