feat(nuxt): Add `entrypointWrappedFunctions` to define async wrapped server functions #14104

s1gr1d · 2024-10-28T16:24:12Z

Based on this PR: #14086

We get the names of the exports with Rollup's exportedBindings, but we cannot know whether this is a function or something else. As we need to re-export serverless functions for Netlify, Vercel and so on, an educated guess is made that 'default', 'handler', 'server' are potential serverless functions that need to be wrapped by Sentry and re-exported.

In case users experience issues, this can be changed with entrypointWrappedFunctions.

All other exports are exported as they were before.

github-actions · 2024-10-28T16:32:27Z

size-limit report 📦

⚠️ Warning: Base artifact is not the latest one, because the latest workflow run is not done yet. This may lead to incorrect results. Try to re-run all tests to get up to date results.

Path	Size	% Change	Change
@sentry/browser	22.74 KB	-	-
@sentry/browser - with treeshaking flags	21.53 KB	-	-
@sentry/browser (incl. Tracing)	35.14 KB	-	-
@sentry/browser (incl. Tracing, Replay)	71.86 KB	-	-
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags	62.29 KB	-	-
@sentry/browser (incl. Tracing, Replay with Canvas)	76.17 KB	-	-
@sentry/browser (incl. Tracing, Replay, Feedback)	88.97 KB	-	-
@sentry/browser (incl. Tracing, Replay, Feedback, metrics)	90.81 KB	-	-
@sentry/browser (incl. metrics)	26.99 KB	-	-
@sentry/browser (incl. Feedback)	39.89 KB	-	-
@sentry/browser (incl. sendFeedback)	27.39 KB	-	-
@sentry/browser (incl. FeedbackAsync)	32.18 KB	-	-
@sentry/react	25.5 KB	-	-
@sentry/react (incl. Tracing)	38.1 KB	-	-
@sentry/vue	26.89 KB	-	-
@sentry/vue (incl. Tracing)	37.02 KB	-	-
@sentry/svelte	22.88 KB	-	-
CDN Bundle	24.1 KB	-	-
CDN Bundle (incl. Tracing)	36.96 KB	-	-
CDN Bundle (incl. Tracing, Replay)	71.62 KB	-	-
CDN Bundle (incl. Tracing, Replay, Feedback)	76.96 KB	-	-
CDN Bundle - uncompressed	70.64 KB	-	-
CDN Bundle (incl. Tracing) - uncompressed	109.71 KB	-	-
CDN Bundle (incl. Tracing, Replay) - uncompressed	222.23 KB	-	-
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed	235.45 KB	-	-
@sentry/nextjs (client)	38.18 KB	-	-
@sentry/sveltekit (client)	35.75 KB	-	-
@sentry/node	129.6 KB	+0.01%	+1 B 🔺
@sentry/node - without tracing	94.3 KB	-	-
@sentry/aws-serverless	105.17 KB	-0.01%	-1 B 🔽

View base workflow run

mydea · 2024-10-29T08:01:33Z

packages/nuxt/src/common/types.ts

+   * The `asyncFunctionReExports` option is only relevant when `dynamicImportForServerEntry: true` (default value).
+   *
+   * As the server entry file is wrapped with a dynamic `import()`, previous async function exports need to be re-exported.
+   * The SDK detects and re-exports those exports (mostly serverless functions). This is why they are re-exported as async functions.
+   * In case you have a custom setup and your server exports other async functions, you can override the default array with this option.


l: Maybe we can rewrite this a bit, to a perspective of somebody who does not have deep understanding of all of this :D

Suggested change

* The `asyncFunctionReExports` option is only relevant when `dynamicImportForServerEntry: true` (default value).

*

* As the server entry file is wrapped with a dynamic `import()`, previous async function exports need to be re-exported.

* The SDK detects and re-exports those exports (mostly serverless functions). This is why they are re-exported as async functions.

* In case you have a custom setup and your server exports other async functions, you can override the default array with this option.

* By default (unless you configure `dynamicImportForServerEntry: false`) the SDK will try to wrap your application entrypoint with a dynamic `import()` to ensure all dependencies can be properly instrumented.

*

* By default, the SDK will wrap the default export as well as a `handler` or `server` export from the entrypoint. If your application has a different main export that is used to run the application, you can overwrite this by providing an array of export names to wrap.

* Any wrapped export is expected to be an async function.

mydea · 2024-10-29T08:02:42Z

packages/nuxt/src/module.ts

+      asyncFunctionReExports: moduleOptionsParam.asyncFunctionReExports
+        ? moduleOptionsParam.asyncFunctionReExports
+        : ['default', 'handler', 'server'],


super-l: I find it slightly easier to read:

Suggested change

asyncFunctionReExports: moduleOptionsParam.asyncFunctionReExports

? moduleOptionsParam.asyncFunctionReExports

: ['default', 'handler', 'server'],

asyncFunctionReExports: moduleOptionsParam.asyncFunctionReExports || ['default', 'handler', 'server'],

packages/nuxt/src/common/types.ts

packages/nuxt/src/vite/utils.ts

mydea · 2024-10-29T08:12:24Z

packages/nuxt/test/vite/utils.test.ts

@@ -111,16 +111,16 @@ describe('constructFunctionReExport', () => {
    const result2 = constructFunctionReExport(query2, entryId);

    const expected = `
-async function reExport(...args) {
+async function reExport0(...args) {


m: Can we also have a test here to show that it does not change/wrap an export that does not match?

That would require changing a lot of logic in the code. The here tested function constructFunctionReExport is only getting the input from the query parameters and creating a function for each query param.

The part, where those query params are actually added is here:

const functionsToExport = flatten(Object.values(moduleInfo.exportedBindings || {})).filter(functionName => asyncFunctionReExports.includes(functionName), );

One thing I could do is extracting this part as a function to test this step as well. But as adding the query params and generating the function code happen in two different Rollup hooks, I cannot combine those two things in one function or test.

lforst · 2024-10-29T09:08:09Z

I'd design the API as follows:

interface WhateverOptions {
  entrypointExportOverrides: {
    asyncFunctions: string[];
    functions: string[];
  }
}

mydea · 2024-10-29T09:20:26Z

I'd design the API as follows:

interface WhateverOptions {
  entrypointExportOverrides: {
    asyncFunctions: string[];
    functions: string[];
  }
}

We cannot wrap functions this way though, can we? Or we'd turn the sync function into an async one (which we already do anyhow right now if we'd match)?

lforst · 2024-10-29T09:22:24Z

I'd design the API as follows:
interface WhateverOptions {
  entrypointExportOverrides: {
    asyncFunctions: string[];
    functions: string[];
  }
}
We cannot wrap functions this way though, can we? Or we'd turn the sync function into an async one (which we already do anyhow right now if we'd match)?

Right, I feel like we need a mechanism that will just exclude certain exports from the whole shebang

mydea · 2024-10-29T12:20:12Z

I'd design the API as follows:
interface WhateverOptions {
  entrypointExportOverrides: {
    asyncFunctions: string[];
    functions: string[];
  }
}
We cannot wrap functions this way though, can we? Or we'd turn the sync function into an async one (which we already do anyhow right now if we'd match)?
Right, I feel like we need a mechanism that will just exclude certain exports from the whole shebang

I think the idea is (Sigrid correct me if I'm wrong):

We do not wrap anything, generally, except the exports defined in this array (all of which are assumed to be async functions). If you do not want to wrap anything you could define [], if you only want to wrap e.g. the myServer export but nothing else you could do ['myServer']. Does that make sense? Any other export will remain untouched (e.g. be exported normally as before, without dynamic import)

s1gr1d · 2024-10-30T15:47:15Z

I think the suggestion of @lforst makes sense because if nothing is re-exported it's not exported at all. The problem here is that exportedBindings from Rollup only provides strings and I cannot know what this export should be. Assuming this will always be serverless functions, it's re-exported as an async function.

… functions

packages/nuxt/src/common/types.ts

packages/nuxt/src/vite/utils.ts

s1gr1d force-pushed the sig/nuxt-reexport-functions branch from 75db7b5 to 031e219 Compare October 28, 2024 16:24

s1gr1d requested review from lforst, mydea and andreiborza October 28, 2024 16:25

mydea reviewed Oct 29, 2024

View reviewed changes

Base automatically changed from sig/nuxt-reexport-firebase to develop October 29, 2024 08:04

andreiborza reviewed Oct 29, 2024

View reviewed changes

packages/nuxt/src/common/types.ts Show resolved Hide resolved

packages/nuxt/src/vite/utils.ts Outdated Show resolved Hide resolved

mydea reviewed Oct 29, 2024

View reviewed changes

s1gr1d added 4 commits October 31, 2024 13:31

feat(nuxt): Add asyncFunctionReExports to define re-exported server…

f2f75d7

… functions

review comments: add test

6a29eba

reexport and wrap functions

9ed9471

reexport and wrap functions

b8a73ca

s1gr1d force-pushed the sig/nuxt-reexport-functions branch from 84ff708 to b8a73ca Compare October 31, 2024 12:31

s1gr1d changed the title ~~feat(nuxt): Add asyncFunctionReExports to define re-exported server functions~~ feat(nuxt): Add entrypointWrappedFunctions to define async wrapped server functions Oct 31, 2024

s1gr1d requested review from mydea and andreiborza October 31, 2024 12:33

s1gr1d self-assigned this Oct 31, 2024

andreiborza reviewed Oct 31, 2024

View reviewed changes

review suggestions

62e95b2

s1gr1d requested a review from andreiborza October 31, 2024 15:29

andreiborza approved these changes Oct 31, 2024

View reviewed changes

s1gr1d merged commit c579a45 into develop Nov 4, 2024
147 checks passed

s1gr1d deleted the sig/nuxt-reexport-functions branch November 4, 2024 08:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(nuxt): Add `entrypointWrappedFunctions` to define async wrapped server functions #14104

feat(nuxt): Add `entrypointWrappedFunctions` to define async wrapped server functions #14104

s1gr1d commented Oct 28, 2024 •

edited

Loading

github-actions bot commented Oct 28, 2024 •

edited

Loading

mydea Oct 29, 2024

mydea Oct 29, 2024

mydea Oct 29, 2024

s1gr1d Oct 30, 2024 •

edited

Loading

lforst commented Oct 29, 2024

mydea commented Oct 29, 2024

lforst commented Oct 29, 2024

mydea commented Oct 29, 2024

s1gr1d commented Oct 30, 2024

feat(nuxt): Add entrypointWrappedFunctions to define async wrapped server functions #14104

feat(nuxt): Add entrypointWrappedFunctions to define async wrapped server functions #14104

Conversation

s1gr1d commented Oct 28, 2024 • edited Loading

github-actions bot commented Oct 28, 2024 • edited Loading

size-limit report 📦

mydea Oct 29, 2024

Choose a reason for hiding this comment

mydea Oct 29, 2024

Choose a reason for hiding this comment

mydea Oct 29, 2024

Choose a reason for hiding this comment

s1gr1d Oct 30, 2024 • edited Loading

Choose a reason for hiding this comment

lforst commented Oct 29, 2024

mydea commented Oct 29, 2024

lforst commented Oct 29, 2024

mydea commented Oct 29, 2024

s1gr1d commented Oct 30, 2024

feat(nuxt): Add `entrypointWrappedFunctions` to define async wrapped server functions #14104

feat(nuxt): Add `entrypointWrappedFunctions` to define async wrapped server functions #14104

s1gr1d commented Oct 28, 2024 •

edited

Loading

github-actions bot commented Oct 28, 2024 •

edited

Loading

s1gr1d Oct 30, 2024 •

edited

Loading