Skip to content

Conversation

@maxkosty
Copy link

@maxkosty maxkosty commented Feb 23, 2024

Hoping to reduce complexity for setting up micro-frontends routing.

Also: I suppose we could just make Matcher argument optional instead of exporting a separate function... that may be a cleaner design

Hoping to reduce complexity for setting up micro-frontends routing.
@maxkosty maxkosty requested review from lforst and timfish February 23, 2024 18:09
@timfish
Copy link
Collaborator

timfish commented Feb 23, 2024

It is a nice simplification!

Rather than add another method, maybe we could just set this as a default for matcher?

@maxkosty
Copy link
Author

@timfish

Rather than add another method, maybe we could just set this as a default for matcher?

Agreed, good point

@lforst lforst changed the title feat(core): simpler API for multiplexed transport (draft/proposal: DO NOT MERGE) feat(core): Add makeSimpleMultiplexedTransport Mar 22, 2024
@github-actions
Copy link
Contributor

github-actions bot commented Mar 22, 2024

size-limit report 📦

Path Size % Change Change
@sentry/browser 23.29 KB - -
@sentry/browser - with treeshaking flags 21.96 KB - -
@sentry/browser (incl. Tracing) 35.79 KB - -
@sentry/browser (incl. Tracing, Replay) 73.01 KB - -
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 63.41 KB - -
@sentry/browser (incl. Tracing, Replay with Canvas) 77.32 KB - -
@sentry/browser (incl. Tracing, Replay, Feedback) 89.81 KB - -
@sentry/browser (incl. Feedback) 40.04 KB - -
@sentry/browser (incl. sendFeedback) 27.89 KB - -
@sentry/browser (incl. FeedbackAsync) 32.68 KB - -
@sentry/react 25.96 KB - -
@sentry/react (incl. Tracing) 38.6 KB - -
@sentry/vue 27.49 KB - -
@sentry/vue (incl. Tracing) 37.63 KB - -
@sentry/svelte 23.45 KB - -
CDN Bundle 24.43 KB - -
CDN Bundle (incl. Tracing) 37.46 KB - -
CDN Bundle (incl. Tracing, Replay) 72.64 KB - -
CDN Bundle (incl. Tracing, Replay, Feedback) 78.01 KB - -
CDN Bundle - uncompressed 71.74 KB - -
CDN Bundle (incl. Tracing) - uncompressed 111.05 KB - -
CDN Bundle (incl. Tracing, Replay) - uncompressed 225.1 KB - -
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 238.32 KB - -
@sentry/nextjs (client) 38.89 KB - -
@sentry/sveltekit (client) 36.29 KB - -
@sentry/node 162.52 KB -0.01% -1 B 🔽
@sentry/node - without tracing 98.71 KB - -
@sentry/aws-serverless 126.38 KB - -

View base workflow run

@lforst lforst changed the title feat(core): Add makeSimpleMultiplexedTransport feat: Add makeSimpleMultiplexedTransport Mar 22, 2024
@lforst
Copy link
Contributor

lforst commented Mar 22, 2024

@realkosty I think a separate function is more clear for now in terms of functionality. I don't think we should overload the multiplexedTransport with functionality.

I cleaned your draft up a bit and added tests. Let me know if this feels like a good API for you - I feel like you have the most experience with implementing it in the wild. Imo this is good to ship otherwise. We should probably also backport this to v7 since this PR is now pointing to v8.

@lforst lforst marked this pull request as ready for review March 22, 2024 09:30
maxkosty added a commit to getsentry/sentry-docs that referenced this pull request Mar 23, 2024
* Simplify/reorganize Micro Frontend Suport

* Update micro-frontend-support.mdx

* default matcher instead of a new method

See: getsentry/sentry-javascript#10798 (comment)

* Update docs/platforms/javascript/common/configuration/micro-frontend-support.mdx

Co-authored-by: vivianyentran <[email protected]>

* Update docs/platforms/javascript/common/configuration/micro-frontend-support.mdx

Co-authored-by: vivianyentran <[email protected]>

* Update docs/platforms/javascript/common/configuration/micro-frontend-support.mdx

Co-authored-by: vivianyentran <[email protected]>

* Update docs/platforms/javascript/common/configuration/micro-frontend-support.mdx

Co-authored-by: vivianyentran <[email protected]>

* Update docs/platforms/javascript/common/configuration/micro-frontend-support.mdx

Co-authored-by: vivianyentran <[email protected]>

* Update docs/platforms/javascript/common/configuration/micro-frontend-support.mdx

Co-authored-by: vivianyentran <[email protected]>

* Update micro-frontend-support.mdx

* Note about calling Sentry.init() only once

* Address Luca's comments in micro-frontend-support.mdx

* Update docs/platforms/javascript/common/configuration/micro-frontend-support.mdx

Co-authored-by: Luca Forstner <[email protected]>

* Update docs/platforms/javascript/common/configuration/micro-frontend-support.mdx

Co-authored-by: vivianyentran <[email protected]>

---------

Co-authored-by: vivianyentran <[email protected]>
Co-authored-by: Luca Forstner <[email protected]>
@lforst
Copy link
Contributor

lforst commented Apr 11, 2024

I think the naming of the function sucks but I will merge this in. We can rename or alias later.

@andreiborza andreiborza requested a review from lforst December 13, 2024 08:59
lforst
lforst previously approved these changes Dec 13, 2024
@lforst lforst dismissed their stale review December 13, 2024 09:02

actually the naming sucks too much for me to comfortably approve this

@andreiborza
Copy link
Member

@lforst what do you want to do about the naming of the function? Any suggestions for renaming?

@lforst
Copy link
Contributor

lforst commented Dec 13, 2024

Well find a better name. Currently I don't have ideas.

@maxkosty
Copy link
Author

nice

@mydea
Copy link
Member

mydea commented May 20, 2025

Can we revisit this, and instead of creating a new transport (and figuring out the naming), just make the matcher argument for makeMultiplexedTransport optional, using the implementation from this PR by default?

So users could do:

{ 
  transport: makeMultiplexedTransport(makeFetchTransport) 
}

to get this outcome? cc @realkosty & @andreiborza

@andreiborza
Copy link
Member

@mydea sounds reasonable to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants