feat(core): Add universal buildMetadata function
#7404
Conversation
Lms24
requested review from
a team,
lforst and
AbhiPrasad
and removed request for
a team
| * @param sdkName name of the SDK (e.g. 'nextjs') | ||
| * @param packageNames list of package names (e.g. ['nextjs', 'react']) | ||
| */ | ||
| export function buildMetadata(options: Options, sdkName: string, packageNames: string[]): void { |
There was a problem hiding this comment.
l: I would suggest either making this function pure (ie. making it return a new obj) or call it something like applyMetadata() because it mutates the passed options object.
There was a problem hiding this comment.
If we move forward with this (see other comments), I think I'm gonna go with applyMetadata for bundle size reasons
|
I attempted this back in Dec here #4196 - but elected not to move forward with it because it didn't give as much bundle size wins as a I thought, but maybe we just go ahead with it anyway. |
My motivation wasn't bundle size but rather that it'd be duplication (triplication) of functionality. I think in terms of bundle size there shouldn't be a significant change 🤔 (although it seems there was last time in your PR 🤔 If it turns out there is still a significant increase, then let's drop it) |
size-limit report 📦
|
|
Is size bot acting up again? This PR shouldn't increase any bundle size. |
|
Ok, size-bot isn't happy about this. Then let's go with duplication - closing this. |
No I think size-bot might actually be right. Before, we could mangle the function name but now it's exported. Also, there's an extra param that wasn't there before. Although I'm surprised myself about the drastic increase... but it correlates with #4196 |
While working on adding client and server
initfunctions for the SvelteKit SDK, I noticed that we use utility functions both in Remix and NextJS to add SDK metadata to the init options. These functions are identical and we need the same functionality in SvelteKit as well. Therefore, this PR introduces a universalbuildMetadatafunction in the core package which we can use in all three SDKs (follow-up PR for NextJS and Remix coming soon).Why core? Because we need the
SDK_VERSIONconstant. The alternative would be to move this function to the utils package and pass the version as a parameter. Happy to do this if reviewers prefer it. Otherwise, I think it's fine to leave it in core.ref #7348