fix(next): Fix custom integrations #10220
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
size-limit report 📦
|
mydea
force-pushed
the
fn/nextjs-rewrite-frames
branch
from
99d942f to
ca1a2b9
Compare
mydea
changed the title
mydea
commented
Jan 18, 2024
packages/vercel-edge/src/sdk.ts
Outdated
| if (options.defaultIntegrations === undefined) { | ||
| options.defaultIntegrations = sdkDefaultIntegrations; | ||
| } | ||
| options.defaultIntegrations = |
Member
Author
There was a problem hiding this comment.
Aligning this with e.g. the node SDK - otherwise, the RequestData() integration is not added to the defaults if somebody extends this (like in nextjs).
lforst
approved these changes
Jan 18, 2024
Lms24
approved these changes
Jan 18, 2024
mydea
force-pushed
the
fn/nextjs-rewrite-frames
branch
2 times, most recently
from
31298a3 to
d113796
Compare
Member
Author
|
Tests here are failing because of #10243, once this is merged we can fix this. |
Member
|
adding merge commit so we can include with next release |
mydea
force-pushed
the
fn/nextjs-rewrite-frames
branch
from
75e43af to
4c6baed
Compare
The usage of this was not really working well to begin with, and even worse with the new functional integrations.
mydea
force-pushed
the
fn/nextjs-rewrite-frames
branch
from
4c6baed to
2d1607e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The usage of this was not really working well to begin with, and even worse with the new functional integrations. Because if the user adds the integration themselves (e.g.
integrations: [new RewriteFrames()]), it will not actually get the correct iteratee at all.Overall it is much cleaner anyhow to just fork the integrations properly and use them instead of the default one - then we can rely on the standard behavior of merging integrations etc.
We need to do the same for basically all usages of
addOrUpdateIntegration, as that actually does not work at all anymore with the functional integrations 😬 (and in many instances never really worked properly if users passed in a custom integration themselves).