feat: Delete @sentry/minimal #4971
Merged
Conversation
This file contains 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
Removes all references to `@sentry/apm`, a deprecated package we do not use anymore.
Removes the deprecated `API` class.
Removes `getActiveDomain` function and corresponding type.
The `user` dsn component was renamed to `publicKey`. This PR removes that from the dsn field.
…ntOutgoingRequests (#4859)
Increases the creation timeout of `MongoMemoryServer` to temporarily fix Node integration test flakiness
This PR removes the previously deprecated `frameContextLines` from `NodeOptions`.
These exports were historically used in `@sentry/electron`, but are no longer being used by the Electron SDK or the React Native SDK, so they can be removed.
This updates the SDK to use Typescript 3.8.3, in order to be able to use type-only imports and exports[1]. (These are needed for us to turn on `isolatedModules`, which is in turn is needed for us to switch our build tool away from `tsc`[2], since no other tool understands the relationship between files.)
As a result of this change, a few of the browser integration tests needed to be fixed so that all promises were explicitly awaited, a point about which the linter in 3.8 complains.
This is a breaking change for anyone using TS 3.7.x (there's no one using TS < 3.7.x, since that's our current minimum). That said, though there are plenty of public projects on GH using TS 3.7 and `@sentry/xyz`, if you restrict it to projects using TS 3.7 and `@sentry/xyz` 6.x, all you get are forks of this very repo. Granted, this isn't every project ever, but it's likely decently representative of the fact that if you've upgraded our SDK, you've almost certainly upgraded TS as well. We're going to wait until v7 to release this change in any case, but that's an indication that it won't affect many people when we do. (See this commit's PR for links to searches demonstrating the points.)
Note: Originally there was some thought of going farther, into TS 4.x, but we decided that for the foreseeable future, we're going to stick with 3.8.3. Though moving up to 4.x doesn't seem like it would affect many customers (repeating the same TS/sentry 6.x crossover search with TS 3.8 and 3.9 reveals a total of 5 projects), it does have the known side effect of replacing export statements which, after compilation, currently look like
`exports.SdkInfo = types_1.SdkInfo;`
with ones that look like
`Object.defineProperty(exports, "SdkInfo", { enumerable: true, get: function () { return types_1.SdkInfo; } });`.
(For those of you following along at home, that's a 3x character increase.) Though we might be able to engineer around this in order to avoid the inevitable substantial bundle size hit, attempting to do so is only worth it if there are features from 4.x that we desperately need. Given that we've agreed that right now there aren't, we'll stick with 3.8.3.
[1] https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-8.html#type-only-imports-and-export
[2] https://www.typescriptlang.org/tsconfig#isolatedModules
rename the dist directories in all build dirs to cjs. Hence, also the tarballs' structure changes which is why this PR also introduces a breaking change. Additional change: cleanup `yarn clean` commands by removing no longer existing directories
In #4842, a `flags.ts` file was added to each package to fix logger treeshaking when using webpack. Because of the way that webpack works, this file has to exist separately in each individual package, including in `@sentry/integrations`. It is not itself an integration, though, so we shouldn't be building a separate CDN bundle for it (let alone six versions of one). This fixes the build script so that we're no longer doing that.
Now that we have files like `rollup.config.js` and `jest.config.js` at the package root level, it's helpful if running `yarn clean` doesn't delete them. This fixes that problem, and also fixes a spot where the now-defunct package-top-level `esm` directory was still included in a `clean` script.
The `.gitignore` file in the `utils` package isn't ignoring any current files which the main `.gitignore` (the one at the root level of the repo) isn't already ignoring, as evidenced by the fact that deleting the former doesn't cause anything to new to come to git's attention. What it will ignore, however, is future `.js` files (such as `rollup.config.js` and `jest.config.js`) which we don't want ignored (and which aren't ignored in any other package). This therefore removes the `.gitignore` in `utils`, in order to prevent that future problem.
Currently, we configure jest using a mix of `package.json` entries and `jest.config.js` files, and there's a great deal of repetition between configs. To make things DRYer and easier to update (as will happen in future PRs), this aims to fix that by creating a centralized jest config from which all others can inherit. To facilitate this inheritance, all config has been removed from `package.json` files and moved into `jest.config.js` files. This change was done in a few distinct stages: - Extracting the config which was identical across many packages into a central config file, and fixing a mistake they all contained, namely that they were using the regular tsconfig file rather than the test-specific one. - In the packages which were using exactly that config, creating a new `jest.config.js` file and inheriting directly from the central jest config with no changes. - In the packages whose config varied only slightly from the boilerplate config, creating a new `jest.config.js` file and inheriting from the central file with small changes. This also required adding `.tsx` files to the central config. - In the browser package, moving the existing `jest.config.js` for the unit tests to the repo root level and refactoring it to inherit from the central file. This also required specifying a coverage directory in the central config and modifying the browser package's yarn test commands. - In the node integration test package, refactoring the existing `jest.config.js` to inherit from the central file. This also required creating a test-specific tsconfig to match the one in other packages. - Finally, making a small optimization (narrowing the scope of where to look for tests) in the now universally-used central config.
Documents: - ES6 for CJS files - Dropping Support for Node v6 - Removing Platform Integrations - New npm package structure - Deleting deprecations
Our old browser integration tests are difficult to debug. This adds a file of (hard-won) debugging tips to the test directory, to hopefully make it easier on the next person. While it's true that it contains a lot of very specific references (to functions, files, etc) and is therefore much more susceptible to becoming out of date, these tests aren't something we change often, and the consequences of such staleness are small.
Removes usage of deprecated `event.stacktrace` and updated the event interface to remove the deprecated property.
AbhiPrasad
requested review from
a team,
lforst,
lobsterkatie and
kamilogorek
and removed request for
a team
size-limit report 📦
|
AbhiPrasad
force-pushed
the
v7-abhi-delete-minimal
branch
from
02af0fa
to
215d57d
Compare
lforst
approved these changes
Apr 26, 2022
| export function configureScope(callback: (scope: Scope) => void): void { | ||
| callOnHub<void>('configureScope', callback); | ||
| export function configureScope(callback: (scope: Scope) => void): ReturnType<Hub['configureScope']> { | ||
| getCurrentHub().configureScope(callback); |
There was a problem hiding this comment.
Gotta say I'm not a huge fan of these return types. Imo it would be better to just define the type right here (void in this case) and add a return for good measure, even though it's a void func, but we want these functions to be legit proxies I believe.
I'll let you make the last call on this however.
There was a problem hiding this comment.
Agree with the return, but I'd leave the type as is. This way we are 100% sure that they are in sync.
There was a problem hiding this comment.
So I initially had the return types (see: 3d3dd2b), but I removed because it's unnecessary bundle size (can't minify return). I can add a note about this.
Will add migration docs!
|
Fixed the tests with 8d66cba |
|
cursed rebase 😓 - let's see what we can do! |
lobsterkatie
pushed a commit
that referenced
this pull request
Apr 26, 2022
Remove the minimal package and `callOnHub`. Instead, call hub methods directly. This gives static typing and bundle size improvements.
lobsterkatie
pushed a commit
that referenced
this pull request
Apr 26, 2022
Remove the minimal package and `callOnHub`. Instead, call hub methods directly. This gives static typing and bundle size improvements.
AbhiPrasad
added a commit
that referenced
this pull request
May 30, 2022
Remove the minimal package and `callOnHub`. Instead, call hub methods directly. This gives static typing and bundle size improvements.
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.
Remove the minimal package and
callOnHub. Instead, call hub methods directly. This gives static typing and bundle size improvements.Resolves https://getsentry.atlassian.net/browse/WEB-793.