-
-
Notifications
You must be signed in to change notification settings - Fork 7.5k
build: use isolated declarations #20928
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 6 commits
9383f93
e88c60e
d9c8230
1df000f
c1e2b11
0be956a
e5fbe00
dda8578
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,14 @@ | ||
| // https://gist.github.com/samthor/64b114e4a4f539915a95b91ffd340acc | ||
| // DO NOT ALTER THIS CONTENT | ||
| export const safari10NoModuleFix = `!function(){var e=document,t=e.createElement("script");if(!("noModule"in t)&&"onbeforeload"in t){var n=!1;e.addEventListener("beforeload",(function(e){if(e.target===t)n=!0;else if(!e.target.hasAttribute("nomodule")||!n)return;e.preventDefault()}),!0),t.type="module",t.src=".",e.head.appendChild(t),t.remove()}}();` | ||
| export const safari10NoModuleFix: string = `!function(){var e=document,t=e.createElement("script");if(!("noModule"in t)&&"onbeforeload"in t){var n=!1;e.addEventListener("beforeload",(function(e){if(e.target===t)n=!0;else if(!e.target.hasAttribute("nomodule")||!n)return;e.preventDefault()}),!0),t.type="module",t.src=".",e.head.appendChild(t),t.remove()}}();` | ||
|
|
||
| export const legacyPolyfillId = 'vite-legacy-polyfill' | ||
| export const legacyEntryId = 'vite-legacy-entry' | ||
| export const systemJSInlineCode = `System.import(document.getElementById('${legacyEntryId}').getAttribute('data-src'))` | ||
| export const legacyPolyfillId: string = 'vite-legacy-polyfill' | ||
| export const legacyEntryId: string = 'vite-legacy-entry' | ||
| export const systemJSInlineCode: string = `System.import(document.getElementById('${legacyEntryId}').getAttribute('data-src'))` | ||
|
|
||
| const detectModernBrowserVarName = '__vite_is_modern_browser' | ||
| export const detectModernBrowserDetector = `import.meta.url;import("_").catch(()=>1);(async function*(){})().next()` | ||
| export const detectModernBrowserCode = `${detectModernBrowserDetector};window.${detectModernBrowserVarName}=true` | ||
| export const dynamicFallbackInlineCode = `!function(){if(window.${detectModernBrowserVarName})return;console.warn("vite: loading legacy chunks, syntax error above and the same error below should be ignored");var e=document.getElementById("${legacyPolyfillId}"),n=document.createElement("script");n.src=e.src,n.onload=function(){${systemJSInlineCode}},document.body.appendChild(n)}();` | ||
| export const detectModernBrowserDetector: string = `import.meta.url;import("_").catch(()=>1);(async function*(){})().next()` | ||
| export const detectModernBrowserCode: string = `${detectModernBrowserDetector};window.${detectModernBrowserVarName}=true` | ||
| export const dynamicFallbackInlineCode: string = `!function(){if(window.${detectModernBrowserVarName})return;console.warn("vite: loading legacy chunks, syntax error above and the same error below should be ignored");var e=document.getElementById("${legacyPolyfillId}"),n=document.createElement("script");n.src=e.src,n.onload=function(){${systemJSInlineCode}},document.body.appendChild(n)}();` | ||
|
|
||
| export const modernChunkLegacyGuard = `export function __vite_legacy_guard(){${detectModernBrowserDetector}};` | ||
| export const modernChunkLegacyGuard: string = `export function __vite_legacy_guard(){${detectModernBrowserDetector}};` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| { | ||
| "extends": "../../../tsconfig.base.json", | ||
| "compilerOptions": { | ||
| "isolatedDeclarations": false, | ||
| "declaration": false | ||
| }, | ||
| "include": ["../", "../../types"], | ||
| "exclude": ["../**/__tests__"] | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -354,7 +354,7 @@ export interface ResolvedBuildOptions | |
| modulePreload: false | ResolvedModulePreloadOptions | ||
| } | ||
|
|
||
| export const buildEnvironmentOptionsDefaults = Object.freeze({ | ||
| const _buildEnvironmentOptionsDefaults = Object.freeze({ | ||
| target: 'baseline-widely-available', | ||
| /** @deprecated */ | ||
| polyfillModulePreload: true, | ||
|
|
@@ -390,7 +390,10 @@ export const buildEnvironmentOptionsDefaults = Object.freeze({ | |
| chunkSizeWarningLimit: 500, | ||
| watch: null, | ||
| // createEnvironment | ||
| }) | ||
| } satisfies BuildEnvironmentOptions) | ||
| export const buildEnvironmentOptionsDefaults: Readonly< | ||
| Partial<BuildEnvironmentOptions> | ||
| > = _buildEnvironmentOptionsDefaults | ||
|
Comment on lines
+394
to
+396
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This and other similar setups are kinda funky to handle with isolated declarations, and they all relate to |
||
|
|
||
| export function resolveBuildEnvironmentOptions( | ||
| raw: BuildEnvironmentOptions, | ||
|
|
@@ -414,7 +417,7 @@ export function resolveBuildEnvironmentOptions( | |
|
|
||
| const merged = mergeWithDefaults( | ||
| { | ||
| ...buildEnvironmentOptionsDefaults, | ||
| ..._buildEnvironmentOptionsDefaults, | ||
| cssCodeSplit: !raw.lib, | ||
| minify: consumer === 'server' ? false : 'esbuild', | ||
| ssr: consumer === 'server', | ||
|
|
@@ -1443,8 +1446,10 @@ export function toOutputFilePathWithoutRuntime( | |
| } | ||
| } | ||
|
|
||
| export const toOutputFilePathInCss = toOutputFilePathWithoutRuntime | ||
| export const toOutputFilePathInHtml = toOutputFilePathWithoutRuntime | ||
| export const toOutputFilePathInCss: typeof toOutputFilePathWithoutRuntime = | ||
| toOutputFilePathWithoutRuntime | ||
| export const toOutputFilePathInHtml: typeof toOutputFilePathWithoutRuntime = | ||
| toOutputFilePathWithoutRuntime | ||
|
|
||
| export class BuildEnvironment extends BaseEnvironment { | ||
| mode = 'build' as const | ||
|
|
@@ -1505,18 +1510,20 @@ export interface BuilderOptions { | |
| buildApp?: (builder: ViteBuilder) => Promise<void> | ||
| } | ||
|
|
||
| export const builderOptionsDefaults = Object.freeze({ | ||
| const _builderOptionsDefaults = Object.freeze({ | ||
| sharedConfigBuild: false, | ||
| sharedPlugins: false, | ||
| // buildApp | ||
| }) | ||
| } satisfies BuilderOptions) | ||
| export const builderOptionsDefaults: Readonly<Partial<BuilderOptions>> = | ||
| _builderOptionsDefaults | ||
|
|
||
| export function resolveBuilderOptions( | ||
| options: BuilderOptions | undefined, | ||
| ): ResolvedBuilderOptions | undefined { | ||
| if (!options) return | ||
| return mergeWithDefaults( | ||
| { ...builderOptionsDefaults, buildApp: async () => {} }, | ||
| { ..._builderOptionsDefaults, buildApp: async () => {} }, | ||
| options, | ||
| ) | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about this setting?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting that has 101 errors. I think in some cases
type-annotationmakes sense for exported types with isolated declaration, but nested code that doesn't need it writes better if the type is in theconstructorinstead. Probablytypescript-eslintneeds a new option (or tweakconstructor) that balances for isolated declarations.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. It seems
typescript-eslinthaslanguageOptions.parserOptions.isolatedDeclarations: truefor that purpose.https://github.com/typescript-eslint/typescript-eslint/pull/11351/files#diff-7ca46a3da27e8ad890168dc27f592c83d7f9e12334428396e11b8ca2a6a850adR52-R56
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice find! That option actually seems to work and we don't have to disable this rule