Sentry JavaScript v7 Major Release Plan #4240
Comments
|
Hey everyone! For those watching the issue - here's where we are at. We recently did a bunch of changes in prep for this major, culminating in a beta release: https://www.npmjs.com/package/@sentry/browser/v/6.17.0-beta.0. Please give it a try and let us know what you think! After we've cut a release for the full |
|
Is there any opportunity to add a checkbox to every item listed? It would help to understand what has been done already and what is missing. I think I may be able to do some of the work hopefully. |
Append of Prepend? Normally I would expect |
|
Prefixing is the traditional way for many years in .NET, Java and possibly others. However, the TypeScript team themselves really want people to stop that practise: |
|
@alfaproject my experience doesn't allow me to agree with such a take anymore, I understand your opinion, and totally valid. The countless time people decided to remove the A) A lot of headaches when it comes to ambiguous language and adds a lot of cognitive loads. If you pay close attention, multiple projects are starting to actually move to full names for types instead of using So, TypeScript could Being said, 🤷🏻 to each their own, I don't own the codebase. But if we are honest about the situation, what I proposed wouldn't be causing issues by now and not much to explain the intention 😉 It is not an all or nothing either, don't do it because of Java or C# or whatever of those reasons, but more to fix an exciting problem. |
|
No no you misunderstood me, haha. I just wanted to say no the suffix because prefix is what everyone does including for types like The trying to avoid prefixes was just an addendum that I totally understand is not possible in any enterprise grade project. Hell we also prefix interfaces with |
|
@alfaproject sorry ... (feel stupid now missing your intention) |
|
#3271 Will this bug get fixed in v7? |
RoadmapWe've spent some while experimenting, and we now have a definite road-map for the major Before the MajorBefore we start work on the major, there are a couple things we can do to ease migration, test out ideas, and set up tests. This comes in a couple stages: Introduce New transport API + First Class Envelope SupportOur main goal here is to unblock #4417. We'll do this first by incrementally migrating the Transports from the old API to the new one. This means there will be a small bundle size hit to accommodate the two different transport types, but this will led to a net smaller bundle when the major comes by. #4527 starts work on the envelope side of things. TraceKit RefactorsOur main goal here is to simplify TraceKit to reduce bundle size + allow for arbitrary stack trace parsers to be passed in. This means in the major, you can choose exactly what stack trace parsing you want, so if you want to remove support for safari or firefox, you can tree-shake them out! We'll also be just improving this in general by removing unnecessary intermediate types. The awesome @timfish is helping us move forward with this! For the exact plan, see: #4543 Introduce New integration APIWe want to experiment with a new API for integrations, one that reduces bundle size and helps address some issues that are raised when a user has multiple clients/hubs (we've tracked some of those issues in GitHub) /** Integration interface */
export interface NewIntegration {
// bundle size efficient, represents the name
n: string;
// explicitly pass in hub reference so we don't worry about mutable global state
install(hub: Hub): void;
}Release Stability and Testing - Bundles and NodeWe want to make sure we improve the testing infrastructure we have in place so the major bump goes as smooth as possible. @onurtemizkan has been doing some great work around this! We're in a good place with our current browser integration tests, now we need to do two things.
BundlingIn the major, we plan to start emitting different bundles (for example, a production bundle with all debug logging stripped) to give users control over what exactly they want. To accommodate this, we have the testing work detailed above, but we are also refactoring and centralizing our rollup config (along with some other smaller changes). @lobsterkatie is taking the lead with this so far. Re-arranging typesAs suggested by @yordis above, we'll be prefixing our interfaces with v7 MajorOnce we have all of that in a good place, we'll be putting the SDK in a short feature freeze to focus on the major bump. At the end of each step, we'll be cutting a beta for everyone to test, and then we'll have some release candidate before final release. Part 1: Delete DeprecationsAs detailed in the original comment on this thread, we'll be going in and deleting all the deprecated code and options. This is detailed in the Part 2: Web Bundling OptimizationsHere we bump typescript, start emitting our different bundles, and switch to emitting es6 by default. If you want to use es5, we'll provide a separate npm package, and continue to produce es5 CDN bundles. Part 3: Delete the backend classAs seen in https://github.com/getsentry/sentry-javascript/blob/master/packages/core/src/basebackend.ts. It is an unnecessary abstraction that is part of no other SDK. We'll be moving the logic into client. Early attempt made here: #4307 Part 4: Remove support for the store endpoint + Switch to new transport APISwitch over to using the new transport API detailed in Part 5: Switch to new integrations APIFormalize the switch detailed in We will also extract certain integrations into their own packages:
Part 6: Refactor Class Inheritance Chain (specifically Client)Remove all dead code that comes from overriding class methods. Leverage We can also maybe improve the processing event workflow, and how we create and propogate event ids (#4571) Part 7: Use the Angular compiler to compile @sentry/angularSee #4644 Part 8: Final Touchups
|
Could that be even shorter (by allowing compression of type HubInstall = (hub: Hub)=> void;
type Integration = [name: string, HubInstall];
Have you checked #4381? It is a total mess but kind of proved the point if you want to save every byte possible. I am more than happy to explain the thought process if it didn't make sense. How I feel right now since I am not part of the team.
|
|
@yordis we got open source and open arms bro
and we 100% checked 4381 I think we got the general idea of what you wanted to achieve but I am sure @AbhiPrasad would be happy to get more of your thoughts on it |
|
@yordis #4381 is a great collection of ideas! It's really important to us that we minimize the public API breakage, but maybe we can do some of the class -> functions in the major. We want to make sure we follow this as closely as possible (from the original plan at the top):
This means we can't use the changes to the scope class (because something like With that in mind, I think we can bring in two of your changes. First, we can probably get away with converting the Session entirely to an object + functions (we'll need an incremental migration strategy though, any ideas?). Second, have any ideas on how we can do the Edit:
Turning the integration into a tuple is really interesting actually. Maybe we'll give it a try when we experiment with the integration API, will ping you on the PR! Edit 2: We probably can't rely on a strict tuple because we want integration to be stateful (can be passed in some options in the constructor, but they don't run until they get installed). Perhaps we can just make them plain objects though. |
Generally, if you are gonna make a breaking change anyway, all it matters is the surface to go from A to B, I feel this would simplify either way, so I am biased toward breaking changes as long as people take the ownership document as much you can and provide tools and guidelines. But don't listen to me, I am crazy when it comes to that part.
Right right this, you can always push as much as you can to be OOP at the edge only at least. You can do it OOP from the external consumers (I think that would be the minimal thing), but in the internal codebase, everything is functional, or at least try to be. You kind of doing that already, since you hide behind functions the fact that you eventually communicate to some global object, and people do Or at least push the OOP to that
Be careful, at that point, what you call integration must be a tuple-of-two, so keep adding more callbacks to implement could be a problem, being said, don't be afraid, having such a simple thing shouldn't be an issue.
I am not sure to follow here (so probably I will explain something that you know already 😄) , but don't forget you can use closure to have "stateful" thing (cache), you can either use the file as your scope or the function closure as the scope // sentry code
// just to hide away things and potentially giving people strong type and validation layer
type HubInstall = (hub: Hub)=> void;
type Integration = [name: string, HubInstall];
export function makeIntegration(name: string, installer: HubInstall) {
return [name, installer];
}const fileGlobalStuff = {}
// if you want to have config stuff and extra things or whatever your use-space integration wants.
function makeMyStatefulIntegration(someClient) {
const anotherInternalStuff = {}
return makeIntegration('pepeg', (hub)=> {
// do stuff with fileGlobalStuff
// do stuff with someClient
// do stuff with anotherInternalStuff
})
} |
I agree, but we have users who rely on Sentry in many different environments, workplaces and situations, and migrating for them is often tough. It's also important to note that we haven't done a major like this in a long time - if we had a more steady cadence of majors I think we could way more easily accept these larger changes. I think once we ship this one, we can get more comfortable with more breakage afterwards.
Totally - we adopted this pattern when converting our
a good point - slipped my mind (in fact I did this earlier lol #4283). I think moving to managing state in the lexical scope + tuple returns is what we probably want to target for both our transports api changes and integrations api changes.
Totally fair. I can only really see us introducing 1 other option into the integration install function, which is a Thanks for your thoughts @yordis |
|
@AbhiPrasad FYI for breaking but straightforward to migrate API changes, you may use CodeMods like the React team: https://github.com/reactjs/react-codemod |
|
Hey guys. Any ideas to rewrite the module for node.js in the 7th version?
Thanks. |
|
Hey @xr0master thanks for reaching out. We'll be taking a look at Node right after we ship the major, but removing domains is not in scope for this major version. This is tracked by #4633. I'm curious about this: |
|
@AbhiPrasad there is no issue, it's just that there is a lot of express.js-only code in the |
|
Hi all! We've recently shipped our first beta for the V7 release! We would love to get your feedback on the changes we've made to the SDK and migration guide. Thank you! |
|
v7 has been released! Please reach out if you have any feedback! https://github.com/getsentry/sentry-javascript/releases/tag/7.0.0 |
Update 3: We have released 🚀
https://github.com/getsentry/sentry-javascript/releases/tag/7.0.0
https://www.npmjs.com/package/@sentry/browser/v/7.0.0
https://www.npmjs.com/package/@sentry/node/v/7.0.0
Update 2: There are now beta releases available to test with!
https://www.npmjs.com/package/@sentry/browser/v/next
Migration guide: https://github.com/getsentry/sentry-javascript/blob/7.x/MIGRATION.md
Update: We are now actively working towards the v7 major release. While working on it, we're "freezing" work on
master.Feel free to take a look at our v7 work here: master...7.x
Edit: This plan has now been turned into a proper Roadmap. Please see: #4240 (comment)
Overarching issue for: https://github.com/getsentry/sentry-javascript/milestone/11
Please see: master...7.x
The main goals for this next major release is web bundling optimizations and increased treeshaking support. Bundle size reduction will be a side effect of these changes, but not the explicit goal.
It's also important to note that we will not be breaking any top level public APIs in this major. Anything we touch will be related to internals or to integrations/transports. Therefore unless you are using custom integrations, custom clients or monkey patching our SDK, you will have to make no code changes when upgrading the SDK.
The following is a list of the major breaking changes that we might make in the next major release. We are not committing to including all of these changes in the next release, but any feedback/thoughts are appreciated!
Treeshaking / Bundle Size
Remove
BackendClassRemove
callOnHubandinvokeClientmethods@sentry/minimalExtract out integrations into their own modules
getCurrentHubgetIntegrationcheckdefaultIntegrationswill need to turn into a function (so it can be treeshaken away if needed)Extract out transports into their own modules
Refactor client inheritance chain so that minimal methods are overridden
BrowserClientandNodeClientshould be using as much as possible fromBaseClientabstractmethods inBaseClientv7-dev:sentry-javascript/packages/core/src/baseclient.ts
Line 30 in 6d719e1
sentry-javascript/packages/browser/src/client.ts
Line 37 in 6d719e1
BrowserClientis !!!)Refactor API class into functions
getEnvelopeEndpointWithUrlEncodedAuth) that can't get minifiedRefactor Logger class into functions
Refactor Stacktrace Parsing
tracekit)Emit es6 by default
Dropping Support
Remove store endpoint support → only use envelopes
Remove @sentry/integrations package
@sentry/*)Drop support for Node v6
Removing deprecations
Remove all references to @sentry/apm
sentry-javascript/packages/react/src/profiler.tsx
Line 32 in 4887f98
sentry-javascript/packages/integrations/src/vue.ts
Line 264 in 9c25928
Remove
whitelistUrlsandblacklistUrlsoptionssentry-javascript/packages/browser/src/backend.ts
Line 28 in 4887f98
sentry-javascript/packages/core/src/integrations/inboundfilters.ts
Line 17 in 4887f98
allowUrlsanddenyUrlsspecifically. See https://develop.sentry.dev/inclusion/ for more detailsRemove
getActiveDomainsentry-javascript/packages/hub/src/hub.ts
Line 564 in 4887f98
Remove tracing deprecations (
startSpanandchild)sentry-javascript/packages/types/src/span.ts
Line 135 in 4887f98
sentry-javascript/packages/types/src/hub.ts
Line 186 in 4887f98
Remove Gatsby SDK in
windowRemove
registerRequestInstrumentationnamesentry-javascript/packages/tracing/src/index.ts
Line 12 in 5f5e9ec
Dependency upgrades
Upgrade Typescript (lol)
sentry-javascript/scripts/verify-packages-versions.js
Line 7 in 4887f98
Upgrade to Ember 4.x / ember-auto-import 2.x
Other
Remove
SentryErrorIntroduce a
getActiveTransactionmethod and use it everywheregetActiveTransactionRemove SyncPromise API
Remove
@sentry/wasmin favour of just exporting an integration@sentry/integration-browser-wasmRemove
raven-nodebackward-compat code from node rejection handlerRemove
forgetutilRemove
SDK_NAMEconstants in favor of SDK metadata_metadatavs._internalAPIclass uses_metadataas an argument, so we can't change/remove the concept until we refactor theAPIclass.Inline
injectReportDialogintoshowReportDialogand unify themRemove
helpers.tsfrom@sentry/browserIntroduce
EventTypeconst enum (Low Risk - Low Reward)sentry-javascript/packages/types/src/event.ts
Lines 48 to 52 in 6d719e1
const enumtransactionerrorRemove
event.stacktracefieldImprove eventID propogation
sentry-python's behaviour of creating and passingevent_ids #4065Remove
tslintfrom@sentry-internal/typescriptrm -rfhttps://github.com/getsentry/sentry-javascript/blob/master/packages/typescript/tslint.jsonPrefix interfaces with
Iso they clearly differ from class implementations.IHubinstead ofHubThe text was updated successfully, but these errors were encountered: