From c18f315bd49a977f30caacdd63922204c1d40fd9 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Thu, 3 Jun 2021 07:09:27 +0200 Subject: [PATCH 01/10] Reference types when normalizing options --- src/utils/options/normalizeInputOptions.ts | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/src/utils/options/normalizeInputOptions.ts b/src/utils/options/normalizeInputOptions.ts index 7fdc96de610..e1c7628f9a1 100644 --- a/src/utils/options/normalizeInputOptions.ts +++ b/src/utils/options/normalizeInputOptions.ts @@ -27,10 +27,9 @@ export interface CommandConfigObject { globals: { [id: string]: string } | undefined; } -export function normalizeInputOptions(config: InputOptions): { - options: NormalizedInputOptions; - unsetOptions: Set; -} { +export function normalizeInputOptions( + config: InputOptions +): { options: NormalizedInputOptions; unsetOptions: Set } { // These are options that may trigger special warnings or behaviour later // if the user did not select an explicit value const unsetOptions = new Set(); @@ -39,7 +38,7 @@ export function normalizeInputOptions(config: InputOptions): { const onwarn = getOnwarn(config); const strictDeprecations = config.strictDeprecations || false; const options: NormalizedInputOptions & InputOptions = { - acorn: getAcorn(config) as unknown as NormalizedInputOptions['acorn'], + acorn: (getAcorn(config) as unknown) as NormalizedInputOptions['acorn'], acornInjectPlugins: getAcornInjectPlugins(config), cache: getCache(config), context, @@ -103,7 +102,7 @@ const getAcornInjectPlugins = ( ): NormalizedInputOptions['acornInjectPlugins'] => ensureArray(config.acornInjectPlugins); const getCache = (config: InputOptions): NormalizedInputOptions['cache'] => - (config.cache as unknown as RollupBuild)?.cache || config.cache; + ((config.cache as unknown) as RollupBuild)?.cache || config.cache; const getIdMatcher = >( option: @@ -234,16 +233,7 @@ const getTreeshake = ( if (configTreeshake === false) { return false; } - if (!configTreeshake || configTreeshake === true) { - return { - annotations: true, - moduleSideEffects: () => true, - propertyReadSideEffects: true, - tryCatchDeoptimization: true, - unknownGlobalSideEffects: true - }; - } - if (typeof configTreeshake === 'object') { + if (configTreeshake && typeof configTreeshake === 'object') { if (typeof configTreeshake.pureExternalModules !== 'undefined') { warnDeprecationWithOptions( `The "treeshake.pureExternalModules" option is deprecated. The "treeshake.moduleSideEffects" option should be used instead. "treeshake.pureExternalModules: true" is equivalent to "treeshake.moduleSideEffects: 'no-external'"`, From 8812cd496a09df104992f6d260afe0a5293e4836 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Thu, 3 Jun 2021 08:09:04 +0200 Subject: [PATCH 02/10] Add string presets for the treeshake option --- src/utils/options/mergeOptions.ts | 3 ++ src/utils/options/normalizeInputOptions.ts | 56 +++++++++++++--------- 2 files changed, 36 insertions(+), 23 deletions(-) diff --git a/src/utils/options/mergeOptions.ts b/src/utils/options/mergeOptions.ts index d3b908f0d61..bc3ecb56ea2 100644 --- a/src/utils/options/mergeOptions.ts +++ b/src/utils/options/mergeOptions.ts @@ -129,6 +129,9 @@ function mergeInputOptions( preserveSymlinks: getOption('preserveSymlinks'), shimMissingExports: getOption('shimMissingExports'), strictDeprecations: getOption('strictDeprecations'), + // TODO Lukas implement "preset" form + // TODO Lukas command line string usage overrides this option + // TODO Lukas config strings are changed to the "preset" form treeshake: getObjectOption(config, overrides, 'treeshake', objectifyTreeshakeOption), watch: getWatch(config, overrides, 'watch') }; diff --git a/src/utils/options/normalizeInputOptions.ts b/src/utils/options/normalizeInputOptions.ts index e1c7628f9a1..d57a0fd1e53 100644 --- a/src/utils/options/normalizeInputOptions.ts +++ b/src/utils/options/normalizeInputOptions.ts @@ -233,16 +233,17 @@ const getTreeshake = ( if (configTreeshake === false) { return false; } - if (configTreeshake && typeof configTreeshake === 'object') { - if (typeof configTreeshake.pureExternalModules !== 'undefined') { - warnDeprecationWithOptions( - `The "treeshake.pureExternalModules" option is deprecated. The "treeshake.moduleSideEffects" option should be used instead. "treeshake.pureExternalModules: true" is equivalent to "treeshake.moduleSideEffects: 'no-external'"`, - true, - warn, - strictDeprecations - ); - } - let configWithPreset = configTreeshake; + if (configTreeshake) { + if (typeof configTreeshake === 'object') { + if (typeof configTreeshake.pureExternalModules !== 'undefined') { + warnDeprecationWithOptions( + `The "treeshake.pureExternalModules" option is deprecated. The "treeshake.moduleSideEffects" option should be used instead. "treeshake.pureExternalModules: true" is equivalent to "treeshake.moduleSideEffects: 'no-external'"`, + true, + warn, + strictDeprecations + ); + } + let configWithPreset = configTreeshake; const presetName = configTreeshake.preset; if (presetName) { const preset = treeshakePresets[presetName]; @@ -256,22 +257,31 @@ const getTreeshake = ( ) ); } - } - return { - annotations: configWithPreset.annotations !== false, - moduleSideEffects: configTreeshake.pureExternalModules - ? getHasModuleSideEffects( - configTreeshake.moduleSideEffects, - configTreeshake.pureExternalModules - ) + }return { + annotations: configWithPreset.annotations !== false, + moduleSideEffects: configTreeshake.pureExternalModules + ?getHasModuleSideEffects( + configTreeshake.moduleSideEffects, + configTreeshake.pureExternalModules + ) : getHasModuleSideEffects(configWithPreset.moduleSideEffects, undefined), - propertyReadSideEffects: - configWithPreset.propertyReadSideEffects === 'always' + propertyReadSideEffects: + configWithPreset.propertyReadSideEffects === 'always' ? 'always' : configWithPreset.propertyReadSideEffects !== false, - tryCatchDeoptimization: configWithPreset.tryCatchDeoptimization !== false, - unknownGlobalSideEffects: configWithPreset.unknownGlobalSideEffects !== false - }; + tryCatchDeoptimization: configWithPreset.tryCatchDeoptimization !== false, + unknownGlobalSideEffects: configWithPreset.unknownGlobalSideEffects !== false + }; + } + if (configTreeshake === 'smallest') { + return { + annotations: true, + moduleSideEffects: () => false, + propertyReadSideEffects: false, + tryCatchDeoptimization: false, + unknownGlobalSideEffects: false + }; + } } const preset = treeshakePresets[configTreeshake]; if (preset) { From ffad819130eadf9299b9b90c7be4b87d00c173e7 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Sun, 6 Jun 2021 07:54:04 +0200 Subject: [PATCH 03/10] Add "recommended" option and warn for unknown options --- src/utils/options/normalizeInputOptions.ts | 113 ++++++++++-------- .../treeshake-presets/smallest/_config.js | 1 + .../samples/treeshake-presets/true/_config.js | 1 + .../samples/unknown-treeshake-value/dep.js | 1 + 4 files changed, 65 insertions(+), 51 deletions(-) create mode 100644 test/function/samples/unknown-treeshake-value/dep.js diff --git a/src/utils/options/normalizeInputOptions.ts b/src/utils/options/normalizeInputOptions.ts index d57a0fd1e53..dcb0378ddc5 100644 --- a/src/utils/options/normalizeInputOptions.ts +++ b/src/utils/options/normalizeInputOptions.ts @@ -224,6 +224,36 @@ const getPreserveModules = ( return configPreserveModules; }; +type ObjectValue = Base extends Record ? Base : never; + +const treeshakePresets: { + [key in NonNullable< + ObjectValue['preset'] + >]: NormalizedInputOptions['treeshake']; +} = { + recommended: { + annotations: true, + moduleSideEffects: () => true, + propertyReadSideEffects: true, + tryCatchDeoptimization: true, + unknownGlobalSideEffects: false + }, + safest: { + annotations: true, + moduleSideEffects: () => true, + propertyReadSideEffects: true, + tryCatchDeoptimization: true, + unknownGlobalSideEffects: true + }, + smallest: { + annotations: true, + moduleSideEffects: () => false, + propertyReadSideEffects: false, + tryCatchDeoptimization: false, + unknownGlobalSideEffects: false + } +}; + const getTreeshake = ( config: InputOptions, warn: WarningHandler, @@ -233,55 +263,36 @@ const getTreeshake = ( if (configTreeshake === false) { return false; } - if (configTreeshake) { - if (typeof configTreeshake === 'object') { - if (typeof configTreeshake.pureExternalModules !== 'undefined') { - warnDeprecationWithOptions( - `The "treeshake.pureExternalModules" option is deprecated. The "treeshake.moduleSideEffects" option should be used instead. "treeshake.pureExternalModules: true" is equivalent to "treeshake.moduleSideEffects: 'no-external'"`, - true, - warn, - strictDeprecations - ); - } - let configWithPreset = configTreeshake; - const presetName = configTreeshake.preset; - if (presetName) { - const preset = treeshakePresets[presetName]; - if (preset) { - configWithPreset = { ...preset, ...configTreeshake }; - } else { - error( - errInvalidOption( - 'treeshake.preset', - `valid values are ${printQuotedStringList(Object.keys(treeshakePresets))}` - ) - ); - } - }return { - annotations: configWithPreset.annotations !== false, - moduleSideEffects: configTreeshake.pureExternalModules - ?getHasModuleSideEffects( - configTreeshake.moduleSideEffects, - configTreeshake.pureExternalModules - ) - : getHasModuleSideEffects(configWithPreset.moduleSideEffects, undefined), - propertyReadSideEffects: - configWithPreset.propertyReadSideEffects === 'always' - ? 'always' - : configWithPreset.propertyReadSideEffects !== false, - tryCatchDeoptimization: configWithPreset.tryCatchDeoptimization !== false, - unknownGlobalSideEffects: configWithPreset.unknownGlobalSideEffects !== false - }; - } - if (configTreeshake === 'smallest') { - return { - annotations: true, - moduleSideEffects: () => false, - propertyReadSideEffects: false, - tryCatchDeoptimization: false, - unknownGlobalSideEffects: false - }; + if (!configTreeshake || configTreeshake === true) { + return { + annotations: true, + moduleSideEffects: () => true, + propertyReadSideEffects: true, + tryCatchDeoptimization: true, + unknownGlobalSideEffects: true + }; + } + if (typeof configTreeshake === 'object') { + if (typeof configTreeshake.pureExternalModules !== 'undefined') { + warnDeprecationWithOptions( + `The "treeshake.pureExternalModules" option is deprecated. The "treeshake.moduleSideEffects" option should be used instead. "treeshake.pureExternalModules: true" is equivalent to "treeshake.moduleSideEffects: 'no-external'"`, + true, + warn, + strictDeprecations + ); } + return { + annotations: configTreeshake.annotations !== false, + moduleSideEffects: getHasModuleSideEffects( + configTreeshake.moduleSideEffects, + configTreeshake.pureExternalModules + ), + propertyReadSideEffects: + (configTreeshake.propertyReadSideEffects === 'always' && 'always') || + configTreeshake.propertyReadSideEffects !== false, + tryCatchDeoptimization: configTreeshake.tryCatchDeoptimization !== false, + unknownGlobalSideEffects: configTreeshake.unknownGlobalSideEffects !== false + }; } const preset = treeshakePresets[configTreeshake]; if (preset) { @@ -290,9 +301,9 @@ const getTreeshake = ( error( errInvalidOption( 'treeshake', - `valid values are false, true, ${printQuotedStringList( - Object.keys(treeshakePresets) - )}. You can also supply an object for more fine-grained control` + `please use either ${Object.keys(treeshakePresets) + .map(name => `"${name}"`) + .join(', ')}, false or true` ) ); }; diff --git a/test/form/samples/treeshake-presets/smallest/_config.js b/test/form/samples/treeshake-presets/smallest/_config.js index ac49dae5a46..1c6bc990b67 100644 --- a/test/form/samples/treeshake-presets/smallest/_config.js +++ b/test/form/samples/treeshake-presets/smallest/_config.js @@ -2,6 +2,7 @@ const assert = require('assert'); const path = require('path'); module.exports = { + // solo: true, description: 'handles treeshake preset "smallest"', options: { treeshake: 'smallest', diff --git a/test/form/samples/treeshake-presets/true/_config.js b/test/form/samples/treeshake-presets/true/_config.js index 9a024352632..7f23a56b927 100644 --- a/test/form/samples/treeshake-presets/true/_config.js +++ b/test/form/samples/treeshake-presets/true/_config.js @@ -2,6 +2,7 @@ const assert = require('assert'); const path = require('path'); module.exports = { + // solo: true, description: 'handles treeshake preset true', options: { treeshake: true, diff --git a/test/function/samples/unknown-treeshake-value/dep.js b/test/function/samples/unknown-treeshake-value/dep.js new file mode 100644 index 00000000000..b74a9837c07 --- /dev/null +++ b/test/function/samples/unknown-treeshake-value/dep.js @@ -0,0 +1 @@ +console.log('dep'); From a9cc76b250cb8e2f290bb57f7ff03c628de90b94 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Sun, 6 Jun 2021 13:20:29 +0200 Subject: [PATCH 04/10] Add ability to use presets with overrides --- src/utils/options/normalizeInputOptions.ts | 42 +++++++++++++------ .../treeshake-presets/smallest/_config.js | 1 - .../samples/treeshake-presets/true/_config.js | 1 - .../samples/unknown-treeshake-value/dep.js | 1 - 4 files changed, 30 insertions(+), 15 deletions(-) delete mode 100644 test/function/samples/unknown-treeshake-value/dep.js diff --git a/src/utils/options/normalizeInputOptions.ts b/src/utils/options/normalizeInputOptions.ts index dcb0378ddc5..8c82d8a8e49 100644 --- a/src/utils/options/normalizeInputOptions.ts +++ b/src/utils/options/normalizeInputOptions.ts @@ -281,17 +281,35 @@ const getTreeshake = ( strictDeprecations ); } + let configWithPreset = configTreeshake; + const presetName = configTreeshake.preset; + if (presetName) { + const preset = treeshakePresets[presetName]; + if (preset) { + configWithPreset = { ...preset, ...configTreeshake }; + } else { + error( + errInvalidOption( + 'treeshake.preset', + `valid values are ${printQuotedStringList(Object.keys(treeshakePresets))}` + ) + ); + } + } return { - annotations: configTreeshake.annotations !== false, - moduleSideEffects: getHasModuleSideEffects( - configTreeshake.moduleSideEffects, - configTreeshake.pureExternalModules - ), + annotations: configWithPreset.annotations !== false, + moduleSideEffects: configTreeshake.pureExternalModules + ? getHasModuleSideEffects( + configTreeshake.moduleSideEffects, + configTreeshake.pureExternalModules + ) + : getHasModuleSideEffects(configWithPreset.moduleSideEffects, undefined), propertyReadSideEffects: - (configTreeshake.propertyReadSideEffects === 'always' && 'always') || - configTreeshake.propertyReadSideEffects !== false, - tryCatchDeoptimization: configTreeshake.tryCatchDeoptimization !== false, - unknownGlobalSideEffects: configTreeshake.unknownGlobalSideEffects !== false + configWithPreset.propertyReadSideEffects === 'always' + ? 'always' + : configWithPreset.propertyReadSideEffects !== false, + tryCatchDeoptimization: configWithPreset.tryCatchDeoptimization !== false, + unknownGlobalSideEffects: configWithPreset.unknownGlobalSideEffects !== false }; } const preset = treeshakePresets[configTreeshake]; @@ -301,9 +319,9 @@ const getTreeshake = ( error( errInvalidOption( 'treeshake', - `please use either ${Object.keys(treeshakePresets) - .map(name => `"${name}"`) - .join(', ')}, false or true` + `valid values are false, true, ${printQuotedStringList( + Object.keys(treeshakePresets) + )}. You can also supply an object for more fine-grained control` ) ); }; diff --git a/test/form/samples/treeshake-presets/smallest/_config.js b/test/form/samples/treeshake-presets/smallest/_config.js index 1c6bc990b67..ac49dae5a46 100644 --- a/test/form/samples/treeshake-presets/smallest/_config.js +++ b/test/form/samples/treeshake-presets/smallest/_config.js @@ -2,7 +2,6 @@ const assert = require('assert'); const path = require('path'); module.exports = { - // solo: true, description: 'handles treeshake preset "smallest"', options: { treeshake: 'smallest', diff --git a/test/form/samples/treeshake-presets/true/_config.js b/test/form/samples/treeshake-presets/true/_config.js index 7f23a56b927..9a024352632 100644 --- a/test/form/samples/treeshake-presets/true/_config.js +++ b/test/form/samples/treeshake-presets/true/_config.js @@ -2,7 +2,6 @@ const assert = require('assert'); const path = require('path'); module.exports = { - // solo: true, description: 'handles treeshake preset true', options: { treeshake: true, diff --git a/test/function/samples/unknown-treeshake-value/dep.js b/test/function/samples/unknown-treeshake-value/dep.js deleted file mode 100644 index b74a9837c07..00000000000 --- a/test/function/samples/unknown-treeshake-value/dep.js +++ /dev/null @@ -1 +0,0 @@ -console.log('dep'); From 70129a38cae759ec702dfdc08fd80ecae7849fcd Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Mon, 7 Jun 2021 07:40:18 +0200 Subject: [PATCH 05/10] Add CLI support for presets --- src/utils/options/mergeOptions.ts | 3 --- src/utils/options/normalizeInputOptions.ts | 30 ---------------------- 2 files changed, 33 deletions(-) diff --git a/src/utils/options/mergeOptions.ts b/src/utils/options/mergeOptions.ts index bc3ecb56ea2..d3b908f0d61 100644 --- a/src/utils/options/mergeOptions.ts +++ b/src/utils/options/mergeOptions.ts @@ -129,9 +129,6 @@ function mergeInputOptions( preserveSymlinks: getOption('preserveSymlinks'), shimMissingExports: getOption('shimMissingExports'), strictDeprecations: getOption('strictDeprecations'), - // TODO Lukas implement "preset" form - // TODO Lukas command line string usage overrides this option - // TODO Lukas config strings are changed to the "preset" form treeshake: getObjectOption(config, overrides, 'treeshake', objectifyTreeshakeOption), watch: getWatch(config, overrides, 'watch') }; diff --git a/src/utils/options/normalizeInputOptions.ts b/src/utils/options/normalizeInputOptions.ts index 8c82d8a8e49..96cfba38885 100644 --- a/src/utils/options/normalizeInputOptions.ts +++ b/src/utils/options/normalizeInputOptions.ts @@ -224,36 +224,6 @@ const getPreserveModules = ( return configPreserveModules; }; -type ObjectValue = Base extends Record ? Base : never; - -const treeshakePresets: { - [key in NonNullable< - ObjectValue['preset'] - >]: NormalizedInputOptions['treeshake']; -} = { - recommended: { - annotations: true, - moduleSideEffects: () => true, - propertyReadSideEffects: true, - tryCatchDeoptimization: true, - unknownGlobalSideEffects: false - }, - safest: { - annotations: true, - moduleSideEffects: () => true, - propertyReadSideEffects: true, - tryCatchDeoptimization: true, - unknownGlobalSideEffects: true - }, - smallest: { - annotations: true, - moduleSideEffects: () => false, - propertyReadSideEffects: false, - tryCatchDeoptimization: false, - unknownGlobalSideEffects: false - } -}; - const getTreeshake = ( config: InputOptions, warn: WarningHandler, From e0797c67092ff8d4df8b8126dcad2046245c1b66 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Mon, 7 Jun 2021 08:00:24 +0200 Subject: [PATCH 06/10] Add documentation --- docs/999-big-list-of-options.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/999-big-list-of-options.md b/docs/999-big-list-of-options.md index a568fdd6559..465930dab4c 100755 --- a/docs/999-big-list-of-options.md +++ b/docs/999-big-list-of-options.md @@ -1385,7 +1385,7 @@ Default: `true` Whether to apply tree-shaking and to fine-tune the tree-shaking process. Setting this option to `false` will produce bigger bundles but may improve build performance. You may also choose one of three presets that will automatically be updated if new options are added: -* `"smallest"` will choose option values for you to minimize output size as much as possible. This should work for most code bases as long as you do not rely on certain patterns, which are currently: +* `"smallest"` will choose option values for you to minimize output size as much as possible. This should work for most code bases as long as certain as you do not rely on certain patterns, which are currently: * getters with side effects will only be retained if the return value is used (`treeshake.propertyReadSideEffects: false`) * code from imported modules will only be retained if at least one exported value is used (`treeshake.moduleSideEffects: false`) * you should not bundle polyfills that rely on detecting broken builtins (`treeshake.tryCatchDeoptimization: false`) From 3bcbee10cb10741bf9ca714ab72806fb6f0c7497 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Mon, 7 Jun 2021 08:07:55 +0200 Subject: [PATCH 07/10] Fix docs --- docs/999-big-list-of-options.md | 2 +- src/utils/options/normalizeInputOptions.ts | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/docs/999-big-list-of-options.md b/docs/999-big-list-of-options.md index 465930dab4c..a568fdd6559 100755 --- a/docs/999-big-list-of-options.md +++ b/docs/999-big-list-of-options.md @@ -1385,7 +1385,7 @@ Default: `true` Whether to apply tree-shaking and to fine-tune the tree-shaking process. Setting this option to `false` will produce bigger bundles but may improve build performance. You may also choose one of three presets that will automatically be updated if new options are added: -* `"smallest"` will choose option values for you to minimize output size as much as possible. This should work for most code bases as long as certain as you do not rely on certain patterns, which are currently: +* `"smallest"` will choose option values for you to minimize output size as much as possible. This should work for most code bases as long as you do not rely on certain patterns, which are currently: * getters with side effects will only be retained if the return value is used (`treeshake.propertyReadSideEffects: false`) * code from imported modules will only be retained if at least one exported value is used (`treeshake.moduleSideEffects: false`) * you should not bundle polyfills that rely on detecting broken builtins (`treeshake.tryCatchDeoptimization: false`) diff --git a/src/utils/options/normalizeInputOptions.ts b/src/utils/options/normalizeInputOptions.ts index 96cfba38885..7fdc96de610 100644 --- a/src/utils/options/normalizeInputOptions.ts +++ b/src/utils/options/normalizeInputOptions.ts @@ -27,9 +27,10 @@ export interface CommandConfigObject { globals: { [id: string]: string } | undefined; } -export function normalizeInputOptions( - config: InputOptions -): { options: NormalizedInputOptions; unsetOptions: Set } { +export function normalizeInputOptions(config: InputOptions): { + options: NormalizedInputOptions; + unsetOptions: Set; +} { // These are options that may trigger special warnings or behaviour later // if the user did not select an explicit value const unsetOptions = new Set(); @@ -38,7 +39,7 @@ export function normalizeInputOptions( const onwarn = getOnwarn(config); const strictDeprecations = config.strictDeprecations || false; const options: NormalizedInputOptions & InputOptions = { - acorn: (getAcorn(config) as unknown) as NormalizedInputOptions['acorn'], + acorn: getAcorn(config) as unknown as NormalizedInputOptions['acorn'], acornInjectPlugins: getAcornInjectPlugins(config), cache: getCache(config), context, @@ -102,7 +103,7 @@ const getAcornInjectPlugins = ( ): NormalizedInputOptions['acornInjectPlugins'] => ensureArray(config.acornInjectPlugins); const getCache = (config: InputOptions): NormalizedInputOptions['cache'] => - ((config.cache as unknown) as RollupBuild)?.cache || config.cache; + (config.cache as unknown as RollupBuild)?.cache || config.cache; const getIdMatcher = >( option: From e4b840d76982a74e8b95e7e0e01abf8eb9be9620 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Thu, 3 Jun 2021 07:09:27 +0200 Subject: [PATCH 08/10] Reference types when normalizing options --- src/utils/options/normalizeInputOptions.ts | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/src/utils/options/normalizeInputOptions.ts b/src/utils/options/normalizeInputOptions.ts index 7fdc96de610..18362d0e2a3 100644 --- a/src/utils/options/normalizeInputOptions.ts +++ b/src/utils/options/normalizeInputOptions.ts @@ -27,10 +27,9 @@ export interface CommandConfigObject { globals: { [id: string]: string } | undefined; } -export function normalizeInputOptions(config: InputOptions): { - options: NormalizedInputOptions; - unsetOptions: Set; -} { +export function normalizeInputOptions( + config: InputOptions +): { options: NormalizedInputOptions; unsetOptions: Set } { // These are options that may trigger special warnings or behaviour later // if the user did not select an explicit value const unsetOptions = new Set(); @@ -103,7 +102,7 @@ const getAcornInjectPlugins = ( ): NormalizedInputOptions['acornInjectPlugins'] => ensureArray(config.acornInjectPlugins); const getCache = (config: InputOptions): NormalizedInputOptions['cache'] => - (config.cache as unknown as RollupBuild)?.cache || config.cache; + ((config.cache as unknown) as RollupBuild)?.cache || config.cache; const getIdMatcher = >( option: @@ -234,16 +233,7 @@ const getTreeshake = ( if (configTreeshake === false) { return false; } - if (!configTreeshake || configTreeshake === true) { - return { - annotations: true, - moduleSideEffects: () => true, - propertyReadSideEffects: true, - tryCatchDeoptimization: true, - unknownGlobalSideEffects: true - }; - } - if (typeof configTreeshake === 'object') { + if (configTreeshake && typeof configTreeshake === 'object') { if (typeof configTreeshake.pureExternalModules !== 'undefined') { warnDeprecationWithOptions( `The "treeshake.pureExternalModules" option is deprecated. The "treeshake.moduleSideEffects" option should be used instead. "treeshake.pureExternalModules: true" is equivalent to "treeshake.moduleSideEffects: 'no-external'"`, From 1f6d27bb3c7d12efe43f3720cd730f6c8e855195 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Sun, 13 Jun 2021 07:48:45 +0200 Subject: [PATCH 09/10] Add treeshake.correctVarValueBeforeDeclaration option --- src/ast/nodes/Identifier.ts | 10 +++- src/rollup/types.d.ts | 2 + src/utils/options/normalizeInputOptions.ts | 56 ++++++++++--------- src/utils/options/options.ts | 3 + .../samples/treeshake-preset-override/main.js | 3 + .../preset-with-override/_config.js | 1 + .../preset-with-override/main.js | 3 + .../treeshake-presets/recommended/_config.js | 1 + .../treeshake-presets/recommended/main.js | 3 + .../treeshake-presets/safest/_config.js | 1 + .../treeshake-presets/safest/_expected.js | 3 + .../samples/treeshake-presets/safest/main.js | 3 + .../treeshake-presets/smallest/_config.js | 1 + .../treeshake-presets/smallest/main.js | 3 + .../samples/treeshake-presets/true/_config.js | 1 + .../samples/treeshake-presets/true/main.js | 3 + 16 files changed, 70 insertions(+), 27 deletions(-) diff --git a/src/ast/nodes/Identifier.ts b/src/ast/nodes/Identifier.ts index 97a4f560092..d21db0dd645 100644 --- a/src/ast/nodes/Identifier.ts +++ b/src/ast/nodes/Identifier.ts @@ -14,7 +14,7 @@ import LocalVariable from '../variables/LocalVariable'; import Variable from '../variables/Variable'; import * as NodeType from './NodeType'; import SpreadElement from './SpreadElement'; -import { ExpressionEntity, LiteralValueOrUnknown } from './shared/Expression'; +import { ExpressionEntity, LiteralValueOrUnknown, UNKNOWN_EXPRESSION } from './shared/Expression'; import { ExpressionNode, NodeBase } from './shared/Node'; import { PatternNode } from './shared/Pattern'; @@ -45,9 +45,15 @@ export default class Identifier extends NodeBase implements PatternNode { declare(kind: string, init: ExpressionEntity): LocalVariable[] { let variable: LocalVariable; + const { treeshake } = this.context.options; switch (kind) { case 'var': - variable = this.scope.addDeclaration(this, this.context, init, true); + variable = this.scope.addDeclaration( + this, + this.context, + treeshake && treeshake.correctVarValueBeforeDeclaration ? UNKNOWN_EXPRESSION : init, + true + ); break; case 'function': // in strict mode, functions are only hoisted within a scope but not across block scopes diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts index 8c3a17c3e05..a9a146a791a 100644 --- a/src/rollup/types.d.ts +++ b/src/rollup/types.d.ts @@ -478,6 +478,7 @@ type TreeshakingPreset = 'smallest' | 'safest' | 'recommended'; export interface TreeshakingOptions { annotations?: boolean; + correctVarValueBeforeDeclaration?: boolean; moduleSideEffects?: ModuleSideEffectsOption; preset?: TreeshakingPreset; propertyReadSideEffects?: boolean | 'always'; @@ -489,6 +490,7 @@ export interface TreeshakingOptions { export interface NormalizedTreeshakingOptions { annotations: boolean; + correctVarValueBeforeDeclaration: boolean; moduleSideEffects: HasModuleSideEffects; propertyReadSideEffects: boolean | 'always'; tryCatchDeoptimization: boolean; diff --git a/src/utils/options/normalizeInputOptions.ts b/src/utils/options/normalizeInputOptions.ts index 18362d0e2a3..6ac8161cdf1 100644 --- a/src/utils/options/normalizeInputOptions.ts +++ b/src/utils/options/normalizeInputOptions.ts @@ -7,6 +7,7 @@ import { PreserveEntrySignaturesOption, PureModulesOption, RollupBuild, + TreeshakingOptions, WarningHandler } from '../../rollup/types'; import { ensureArray } from '../ensureArray'; @@ -233,7 +234,22 @@ const getTreeshake = ( if (configTreeshake === false) { return false; } - if (configTreeshake && typeof configTreeshake === 'object') { + if (typeof configTreeshake === 'string') { + const preset = treeshakePresets[configTreeshake]; + if (preset) { + return preset; + } + error( + errInvalidOption( + 'treeshake', + `valid values are false, true, ${printQuotedStringList( + Object.keys(treeshakePresets) + )}. You can also supply an object for more fine-grained control` + ) + ); + } + let configWithPreset: TreeshakingOptions = {}; + if (typeof configTreeshake === 'object') { if (typeof configTreeshake.pureExternalModules !== 'undefined') { warnDeprecationWithOptions( `The "treeshake.pureExternalModules" option is deprecated. The "treeshake.moduleSideEffects" option should be used instead. "treeshake.pureExternalModules: true" is equivalent to "treeshake.moduleSideEffects: 'no-external'"`, @@ -242,7 +258,7 @@ const getTreeshake = ( strictDeprecations ); } - let configWithPreset = configTreeshake; + configWithPreset = configTreeshake; const presetName = configTreeshake.preset; if (presetName) { const preset = treeshakePresets[presetName]; @@ -257,34 +273,24 @@ const getTreeshake = ( ); } } - return { - annotations: configWithPreset.annotations !== false, - moduleSideEffects: configTreeshake.pureExternalModules + } + return { + annotations: configWithPreset.annotations !== false, + correctVarValueBeforeDeclaration: configWithPreset.correctVarValueBeforeDeclaration === true, + moduleSideEffects: + typeof configTreeshake === 'object' && configTreeshake.pureExternalModules ? getHasModuleSideEffects( configTreeshake.moduleSideEffects, configTreeshake.pureExternalModules ) : getHasModuleSideEffects(configWithPreset.moduleSideEffects, undefined), - propertyReadSideEffects: - configWithPreset.propertyReadSideEffects === 'always' - ? 'always' - : configWithPreset.propertyReadSideEffects !== false, - tryCatchDeoptimization: configWithPreset.tryCatchDeoptimization !== false, - unknownGlobalSideEffects: configWithPreset.unknownGlobalSideEffects !== false - }; - } - const preset = treeshakePresets[configTreeshake]; - if (preset) { - return preset; - } - error( - errInvalidOption( - 'treeshake', - `valid values are false, true, ${printQuotedStringList( - Object.keys(treeshakePresets) - )}. You can also supply an object for more fine-grained control` - ) - ); + propertyReadSideEffects: + configWithPreset.propertyReadSideEffects === 'always' + ? 'always' + : configWithPreset.propertyReadSideEffects !== false, + tryCatchDeoptimization: configWithPreset.tryCatchDeoptimization !== false, + unknownGlobalSideEffects: configWithPreset.unknownGlobalSideEffects !== false + }; }; const getHasModuleSideEffects = ( diff --git a/src/utils/options/options.ts b/src/utils/options/options.ts index 22cf89d2923..8899b29476f 100644 --- a/src/utils/options/options.ts +++ b/src/utils/options/options.ts @@ -38,6 +38,7 @@ export const treeshakePresets: { } = { recommended: { annotations: true, + correctVarValueBeforeDeclaration: false, moduleSideEffects: () => true, propertyReadSideEffects: true, tryCatchDeoptimization: true, @@ -45,6 +46,7 @@ export const treeshakePresets: { }, safest: { annotations: true, + correctVarValueBeforeDeclaration: true, moduleSideEffects: () => true, propertyReadSideEffects: true, tryCatchDeoptimization: true, @@ -52,6 +54,7 @@ export const treeshakePresets: { }, smallest: { annotations: true, + correctVarValueBeforeDeclaration: false, moduleSideEffects: () => false, propertyReadSideEffects: false, tryCatchDeoptimization: false, diff --git a/test/cli/samples/treeshake-preset-override/main.js b/test/cli/samples/treeshake-preset-override/main.js index 2ef8f761fe7..46a542d6190 100644 --- a/test/cli/samples/treeshake-preset-override/main.js +++ b/test/cli/samples/treeshake-preset-override/main.js @@ -13,3 +13,6 @@ try { } catch {} unknownGlobal; + +if (!foo) console.log('effect'); +var foo = true; diff --git a/test/form/samples/treeshake-presets/preset-with-override/_config.js b/test/form/samples/treeshake-presets/preset-with-override/_config.js index 73bd2448ad1..cf5764aed70 100644 --- a/test/form/samples/treeshake-presets/preset-with-override/_config.js +++ b/test/form/samples/treeshake-presets/preset-with-override/_config.js @@ -11,6 +11,7 @@ module.exports = { plugins: [ { buildStart(options) { + assert.strictEqual(options.treeshake.correctVarValueBeforeDeclaration, false); assert.strictEqual(options.treeshake.propertyReadSideEffects, false); assert.strictEqual(options.treeshake.tryCatchDeoptimization, false); assert.strictEqual(options.treeshake.unknownGlobalSideEffects, true); diff --git a/test/form/samples/treeshake-presets/preset-with-override/main.js b/test/form/samples/treeshake-presets/preset-with-override/main.js index 2ef8f761fe7..46a542d6190 100644 --- a/test/form/samples/treeshake-presets/preset-with-override/main.js +++ b/test/form/samples/treeshake-presets/preset-with-override/main.js @@ -13,3 +13,6 @@ try { } catch {} unknownGlobal; + +if (!foo) console.log('effect'); +var foo = true; diff --git a/test/form/samples/treeshake-presets/recommended/_config.js b/test/form/samples/treeshake-presets/recommended/_config.js index e943af3e2aa..ada92c53ef7 100644 --- a/test/form/samples/treeshake-presets/recommended/_config.js +++ b/test/form/samples/treeshake-presets/recommended/_config.js @@ -8,6 +8,7 @@ module.exports = { plugins: [ { buildStart(options) { + assert.strictEqual(options.treeshake.correctVarValueBeforeDeclaration, false); assert.strictEqual(options.treeshake.propertyReadSideEffects, true); assert.strictEqual(options.treeshake.tryCatchDeoptimization, true); assert.strictEqual(options.treeshake.unknownGlobalSideEffects, false); diff --git a/test/form/samples/treeshake-presets/recommended/main.js b/test/form/samples/treeshake-presets/recommended/main.js index 2ef8f761fe7..46a542d6190 100644 --- a/test/form/samples/treeshake-presets/recommended/main.js +++ b/test/form/samples/treeshake-presets/recommended/main.js @@ -13,3 +13,6 @@ try { } catch {} unknownGlobal; + +if (!foo) console.log('effect'); +var foo = true; diff --git a/test/form/samples/treeshake-presets/safest/_config.js b/test/form/samples/treeshake-presets/safest/_config.js index 35b71dfd8a1..3e5353f5c7e 100644 --- a/test/form/samples/treeshake-presets/safest/_config.js +++ b/test/form/samples/treeshake-presets/safest/_config.js @@ -8,6 +8,7 @@ module.exports = { plugins: [ { buildStart(options) { + assert.strictEqual(options.treeshake.correctVarValueBeforeDeclaration, true); assert.strictEqual(options.treeshake.propertyReadSideEffects, true); assert.strictEqual(options.treeshake.tryCatchDeoptimization, true); assert.strictEqual(options.treeshake.unknownGlobalSideEffects, true); diff --git a/test/form/samples/treeshake-presets/safest/_expected.js b/test/form/samples/treeshake-presets/safest/_expected.js index 3df383b997e..a859fbd4964 100644 --- a/test/form/samples/treeshake-presets/safest/_expected.js +++ b/test/form/samples/treeshake-presets/safest/_expected.js @@ -13,3 +13,6 @@ try { } catch {} unknownGlobal; + +if (!foo) console.log('effect'); +var foo = true; diff --git a/test/form/samples/treeshake-presets/safest/main.js b/test/form/samples/treeshake-presets/safest/main.js index 2ef8f761fe7..46a542d6190 100644 --- a/test/form/samples/treeshake-presets/safest/main.js +++ b/test/form/samples/treeshake-presets/safest/main.js @@ -13,3 +13,6 @@ try { } catch {} unknownGlobal; + +if (!foo) console.log('effect'); +var foo = true; diff --git a/test/form/samples/treeshake-presets/smallest/_config.js b/test/form/samples/treeshake-presets/smallest/_config.js index ac49dae5a46..39e1162c861 100644 --- a/test/form/samples/treeshake-presets/smallest/_config.js +++ b/test/form/samples/treeshake-presets/smallest/_config.js @@ -8,6 +8,7 @@ module.exports = { plugins: [ { buildStart(options) { + assert.strictEqual(options.treeshake.correctVarValueBeforeDeclaration, false); assert.strictEqual(options.treeshake.propertyReadSideEffects, false); assert.strictEqual(options.treeshake.tryCatchDeoptimization, false); assert.strictEqual(options.treeshake.unknownGlobalSideEffects, false); diff --git a/test/form/samples/treeshake-presets/smallest/main.js b/test/form/samples/treeshake-presets/smallest/main.js index 2ef8f761fe7..46a542d6190 100644 --- a/test/form/samples/treeshake-presets/smallest/main.js +++ b/test/form/samples/treeshake-presets/smallest/main.js @@ -13,3 +13,6 @@ try { } catch {} unknownGlobal; + +if (!foo) console.log('effect'); +var foo = true; diff --git a/test/form/samples/treeshake-presets/true/_config.js b/test/form/samples/treeshake-presets/true/_config.js index 9a024352632..0445050db42 100644 --- a/test/form/samples/treeshake-presets/true/_config.js +++ b/test/form/samples/treeshake-presets/true/_config.js @@ -8,6 +8,7 @@ module.exports = { plugins: [ { buildStart(options) { + assert.strictEqual(options.treeshake.correctVarValueBeforeDeclaration, false); assert.strictEqual(options.treeshake.propertyReadSideEffects, true); assert.strictEqual(options.treeshake.tryCatchDeoptimization, true); assert.strictEqual(options.treeshake.unknownGlobalSideEffects, true); diff --git a/test/form/samples/treeshake-presets/true/main.js b/test/form/samples/treeshake-presets/true/main.js index 2ef8f761fe7..46a542d6190 100644 --- a/test/form/samples/treeshake-presets/true/main.js +++ b/test/form/samples/treeshake-presets/true/main.js @@ -13,3 +13,6 @@ try { } catch {} unknownGlobal; + +if (!foo) console.log('effect'); +var foo = true; From 97a6cd2aff135fea47fa66165f15416e4d53c980 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Mon, 14 Jun 2021 06:40:33 +0200 Subject: [PATCH 10/10] Add documentation --- docs/999-big-list-of-options.md | 30 +++++++++++++++++++--- src/utils/options/normalizeInputOptions.ts | 9 ++++--- 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/docs/999-big-list-of-options.md b/docs/999-big-list-of-options.md index a568fdd6559..9db70067512 100755 --- a/docs/999-big-list-of-options.md +++ b/docs/999-big-list-of-options.md @@ -1379,7 +1379,7 @@ Default: `false` If this option is provided, bundling will not fail if bindings are imported from a file that does not define these bindings. Instead, new variables will be created for these bindings with the value `undefined`. #### treeshake -Type: `boolean | "smallest" | "safest" | "recommended" | { annotations?: boolean, moduleSideEffects?: ModuleSideEffectsOption, preset?: "smallest" | "safest" | "recommended", propertyReadSideEffects?: boolean | 'always', tryCatchDeoptimization?: boolean, unknownGlobalSideEffects?: boolean }`
+Type: `boolean | "smallest" | "safest" | "recommended" | { annotations?: boolean, correctVarValueBeforeDeclaration?: boolean, moduleSideEffects?: ModuleSideEffectsOption, preset?: "smallest" | "safest" | "recommended", propertyReadSideEffects?: boolean | 'always', tryCatchDeoptimization?: boolean, unknownGlobalSideEffects?: boolean }`
CLI: `--treeshake`/`--no-treeshake`
Default: `true` @@ -1389,9 +1389,10 @@ Whether to apply tree-shaking and to fine-tune the tree-shaking process. Setting * getters with side effects will only be retained if the return value is used (`treeshake.propertyReadSideEffects: false`) * code from imported modules will only be retained if at least one exported value is used (`treeshake.moduleSideEffects: false`) * you should not bundle polyfills that rely on detecting broken builtins (`treeshake.tryCatchDeoptimization: false`) - * some semantic errors may be swallowed (`treeshake.unknownGlobalSideEffects: false`) -* `"recommended"` should work well for most usage patterns. Some semantic errors may be swallowed, though (`treeshake.unknownGlobalSideEffects: false`) -* `"safest"` tries to be as spec compliant as possible while still providing some basic tree-shaking capabilities. This is currently equivalent to `true` but this may change in the next major version. + * some semantic issues may be swallowed (`treeshake.unknownGlobalSideEffects: false`, `treeshake.correctVarValueBeforeDeclaration: false`) +* `"recommended"` should work well for most usage patterns. Some semantic issues may be swallowed, though (`treeshake.unknownGlobalSideEffects: false`, `treeshake.correctVarValueBeforeDeclaration: false`) +* `"safest"` tries to be as spec compliant as possible while still providing some basic tree-shaking capabilities. +* `true` is equivalent to not specifying the option and will always choose the default value (see below). If you discover a bug caused by the tree-shaking algorithm, please file an issue! Setting this option to an object implies tree-shaking is enabled and grants the following additional options: @@ -1415,6 +1416,27 @@ class Impure { /*@__PURE__*/new Impure(); ``` +**treeshake.correctVarValueBeforeDeclaration**
+Type: `boolean`
+CLI: `--treeshake.correctVarValueBeforeDeclaration`/`--no-treeshake.correctVarValueBeforeDeclaration`
+Default: `false` + +If a variable is assigned a value in its declaration and is never reassigned, Rollup assumes the value to be constant. This is not true if the variable is declared with `var`, however, as those variables can be accessed before their declaration where they will evaluate to `undefined`. +Choosing `true` will make sure Rollup does not make (wrong) assumptions about the value of such variables. Note though that this can have a noticeable negative impact on tree-shaking results. + +```js +// input +if (x) console.log('not executed'); +var x = true; + +// output with treeshake.correctVarValueBeforeDeclaration === false +console.log('not executed'); + +// output with treeshake.correctVarValueBeforeDeclaration === true +if (x) console.log('not executed'); +var x = true; +``` + **treeshake.moduleSideEffects**
Type: `boolean | "no-external" | string[] | (id: string, external: boolean) => boolean`
CLI: `--treeshake.moduleSideEffects`/`--no-treeshake.moduleSideEffects`/`--treeshake.moduleSideEffects no-external`
diff --git a/src/utils/options/normalizeInputOptions.ts b/src/utils/options/normalizeInputOptions.ts index 6ac8161cdf1..a1aa3ff9e64 100644 --- a/src/utils/options/normalizeInputOptions.ts +++ b/src/utils/options/normalizeInputOptions.ts @@ -28,9 +28,10 @@ export interface CommandConfigObject { globals: { [id: string]: string } | undefined; } -export function normalizeInputOptions( - config: InputOptions -): { options: NormalizedInputOptions; unsetOptions: Set } { +export function normalizeInputOptions(config: InputOptions): { + options: NormalizedInputOptions; + unsetOptions: Set; +} { // These are options that may trigger special warnings or behaviour later // if the user did not select an explicit value const unsetOptions = new Set(); @@ -103,7 +104,7 @@ const getAcornInjectPlugins = ( ): NormalizedInputOptions['acornInjectPlugins'] => ensureArray(config.acornInjectPlugins); const getCache = (config: InputOptions): NormalizedInputOptions['cache'] => - ((config.cache as unknown) as RollupBuild)?.cache || config.cache; + (config.cache as unknown as RollupBuild)?.cache || config.cache; const getIdMatcher = >( option: