-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(nuxt): Add base boilerplate for nuxt #12573
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
Conversation
@@ -24,7 +24,6 @@ const INTEGRATION_NAME = 'Vue'; | |||
const _vueIntegration = ((integrationOptions: Partial<VueOptions> = {}) => { | |||
return { | |||
name: INTEGRATION_NAME, | |||
// TODO v8: Remove this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can remove this comment, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup we can remove!
size-limit report 📦
|
packages/nuxt/package.json
Outdated
"@babel/types": "7.20.7", | ||
"@nuxt/module-builder": "0.8.0", | ||
"nuxt": "^3.12.2", | ||
"vite": "^5.0.10" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you don't need this in here, handled by the workspace already afaik
Could we already add some basic unit tests for the sdk init? |
packages/vue/src/sdk.ts
Outdated
@@ -26,5 +27,7 @@ export function init( | |||
...config, | |||
}; | |||
|
|||
applySdkMetadata(config, 'nuxt', ['nuxt', 'vue']); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does not look right, probably a leftover? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!
packages/nuxt/src/module.ts
Outdated
setup(_moduleOptions, _nuxt) { | ||
// Ignore because of `import.meta.url` | ||
// eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
// @ts-ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
m: We should use @ts-expect-error
, we can remove the eslint disable then.
Why is import.meta.url
causing issues? We should change the tsconfig
in this repo to make sure this works.
const config = useRuntimeConfig(); | ||
|
||
// eslint-disable-next-line no-console | ||
console.log('Plugin initialized'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
m: Let's avoid using a console.log
here.
Also we should only log after we call init
. Perhaps we use debug logger?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this log for testing reasons as long as this sdk is in development. But I'll remove it now to make sure I do not forget to remove it later 👍🏻
packages/nuxt/README.md
Outdated
public: { | ||
sentry: { | ||
dsn: env.DSN, | ||
// Additional config |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does options like beforeSend
or adding custom integrations similar work here? If not, we need a solution for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now it looks like this works
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we'll need to also test that this works with importing stuff (e.g. import { httpClientIntegration } from '@sentry/nuxt'
), but we can look at that in follow ups too! (just before we fully launch this)
7ba1162
to
b65ba5d
Compare
Boilerplate which adds a basic wrapper around the vue SDK. Still a lot WIP.
closes #12572