ref: migrate remaining non-component default export values to named#110630
ref: migrate remaining non-component default export values to named#110630JoshuaKGoldberg merged 1 commit intomasterfrom
Conversation
|
🚨 Warning: This pull request contains Frontend and Backend changes! It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently. Have questions? Please ask in the |
5b5a713 to
09b41bc
Compare
09b41bc to
d3ab52c
Compare
| ts_code = f"""// This is generated code. | ||
| // To update it run `getsentry django generate_controlsilo_urls --format=js --output=/path/to/thisfile.ts` | ||
| const patterns: RegExp[] = [ | ||
| export const controlsiloUrlPatterns: RegExp[] = [ |
There was a problem hiding this comment.
[Explanation] controlsiloUrlPatterns.ts is checked in and linted. We could either stop linting it or update this Python script that generates it.
There was a problem hiding this comment.
we should probably do both.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Removing default fallback breaks unmigrated dynamic imports
- Restored the defensive fallback pattern to check module?.docs first, then module?.default, ensuring dynamically imported modules work regardless of export style.
Or push these changes by commenting:
@cursor push 0bca1bf0c1
Preview (0bca1bf0c1)
diff --git a/static/app/components/onboarding/gettingStartedDoc/utils/useLoadGettingStarted.ts b/static/app/components/onboarding/gettingStartedDoc/utils/useLoadGettingStarted.ts
--- a/static/app/components/onboarding/gettingStartedDoc/utils/useLoadGettingStarted.ts
+++ b/static/app/components/onboarding/gettingStartedDoc/utils/useLoadGettingStarted.ts
@@ -85,7 +85,10 @@
refetch: projectKeys.refetch,
isLoading: projectKeys.isPending || module === undefined,
isError: projectKeys.isError,
- docs: module === 'none' ? null : (module?.docs ?? null),
+ docs:
+ module === 'none'
+ ? null
+ : (module?.docs ?? (module as {default?: Docs<any>})?.default ?? null),
dsn: projectKeys.data?.[0]?.dsn,
projectKeyId: projectKeys.data?.[0]?.id,
};This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
static/app/components/onboarding/gettingStartedDoc/utils/useLoadGettingStarted.ts
Show resolved
Hide resolved
| const [module, setModule] = useState<undefined | 'none' | {default: Docs<any>}>( | ||
| undefined | ||
| ); | ||
| const [module, setModule] = useState<undefined | 'none' | {docs: Docs<any>}>(undefined); |
There was a problem hiding this comment.
[Explanation] This fun little oddity imports modules in a way neither TypeScript nor Rspack/Webpack appreciate:
Tl;dr (see the next change in the file): the modules were previously allowed to have either .default or .docs. I removed all the .defaults so now it's just .docs.
There was a problem hiding this comment.
for the future: is there a way to do this in a not-so-dynamic way? knip is also struggling with this, which is why we have this exception:
Lines 12 to 13 in c1887e1
We also need to know that we have to export a docs const, if we export something else it will not yield a compile time error.
I guess a huge exhaustive switch statement would work, but maybe you know of something better?
There was a problem hiding this comment.
That's just about what I'd do 😞 a big switch statement or an equivalent well-typed object. If we were on Vite we could so something like an import.glob, or in Webpack-land maybe a require.context / webpackInclude might work. But in general trying to base anything in builder+types-land based on file system details is just a weirdness to me.
This comment was marked as outdated.
This comment was marked as outdated.
d3ab52c to
81b4431
Compare
| ts_code = f"""// This is generated code. | ||
| // To update it run `getsentry django generate_controlsilo_urls --format=js --output=/path/to/thisfile.ts` | ||
| const patterns: RegExp[] = [ | ||
| export const controlsiloUrlPatterns: RegExp[] = [ |
There was a problem hiding this comment.
we should probably do both.
| const [module, setModule] = useState<undefined | 'none' | {default: Docs<any>}>( | ||
| undefined | ||
| ); | ||
| const [module, setModule] = useState<undefined | 'none' | {docs: Docs<any>}>(undefined); |
There was a problem hiding this comment.
for the future: is there a way to do this in a not-so-dynamic way? knip is also struggling with this, which is why we have this exception:
Lines 12 to 13 in c1887e1
We also need to know that we have to export a docs const, if we export something else it will not yield a compile time error.
I guess a huge exhaustive switch statement would work, but maybe you know of something better?
81b4431 to
0f5a72e
Compare
0f5a72e to
c63745e
Compare
c63745e to
047ab50
Compare
…t(-components) rule (#110631) Recreates specifically the lint rule expansion from #110517. This: * Removes `isReactComponentLike`, allowing the rule to operate on other values * Renames the rule from `no-default-export-components` to `no-default-exports` Note that the rule still only looks at "straightforward" exports: those where there's only a default export in a file, not any named ones. In `static/app` excluding mocks, this reduces: * `export default \w+`: from ~900 to ~800 * `import \w+ from`: from ~5.8k to ~5.6k Stacked on #110630, which applies the rest of the changes from #110517. Fixes ENG-7049


Splits out the source & test changes from #110517. That PR expands the
no-default-export-componentslint rule to apply to non-component-likes and renames it tono-default-exports.The lint rule expansion itself is in #110631.
In
static/appexcluding mocks, this reduces:export default \w+: from ~900 to ~800import \w+ from: from ~5.8k to ~5.6kFixes ENG-7040.