From 9a669f69baef836ddebc759e245a4a84aaa99cc5 Mon Sep 17 00:00:00 2001 From: Jake Teton-Landis Date: Mon, 18 May 2020 11:14:26 -0700 Subject: [PATCH 1/6] buildOptions: smart merge babelOptions plugins & items --- src/__tests__/evaluators/buildOptions.test.ts | 84 ++++++++ src/babel/evaluators/buildOptions.ts | 193 +++++++++++++----- 2 files changed, 231 insertions(+), 46 deletions(-) create mode 100644 src/__tests__/evaluators/buildOptions.test.ts diff --git a/src/__tests__/evaluators/buildOptions.test.ts b/src/__tests__/evaluators/buildOptions.test.ts new file mode 100644 index 000000000..d6f7c51aa --- /dev/null +++ b/src/__tests__/evaluators/buildOptions.test.ts @@ -0,0 +1,84 @@ +import { + mergeOrAppendPlugin, + mergeOrPrependPlugin, +} from '../../babel/evaluators/buildOptions'; + +describe('mergeOrPrependPlugin', () => { + it('when no duplicates, prepends the new target', () => { + const result = mergeOrPrependPlugin( + ['bogus/already-in-plugins'], + 'bogus/add-to-plugin' + ); + expect(result).toEqual(['bogus/add-to-plugin', 'bogus/already-in-plugins']); + }); + + it('when duplicated, merges options, preferring the new plugin, and moves to the front', () => { + const result = mergeOrPrependPlugin( + [ + 'bogus/already-in-plugins', + [ + 'bogus/add-to-plugin', + { + existing: 'should not be overwritten', + overwrite: 'should be overwritten', + }, + ], + ], + [ + 'bogus/add-to-plugin', + { overwrite: 'was overwritten', added: 'was added' }, + ] + ); + expect(result).toEqual([ + [ + 'bogus/add-to-plugin', + { + existing: 'should not be overwritten', + overwrite: 'was overwritten', + added: 'was added', + }, + ], + 'bogus/already-in-plugins', + ]); + }); +}); + +describe('mergeOrAppendPlugin', () => { + it('when no duplicates, appends the new target', () => { + const result = mergeOrAppendPlugin( + ['bogus/already-in-plugins'], + 'bogus/add-to-plugin' + ); + expect(result).toEqual(['bogus/already-in-plugins', 'bogus/add-to-plugin']); + }); + + it('when duplicated, merges options, preferring the new plugin, and moves to the front', () => { + const result = mergeOrAppendPlugin( + [ + [ + 'bogus/add-to-plugin', + { + existing: 'should not be overwritten', + overwrite: 'should be overwritten', + }, + ], + 'bogus/already-in-plugins', + ], + [ + 'bogus/add-to-plugin', + { overwrite: 'was overwritten', added: 'was added' }, + ] + ); + expect(result).toEqual([ + 'bogus/already-in-plugins', + [ + 'bogus/add-to-plugin', + { + existing: 'should not be overwritten', + overwrite: 'was overwritten', + added: 'was added', + }, + ], + ]); + }); +}); diff --git a/src/babel/evaluators/buildOptions.ts b/src/babel/evaluators/buildOptions.ts index c8602f15c..73e74428e 100644 --- a/src/babel/evaluators/buildOptions.ts +++ b/src/babel/evaluators/buildOptions.ts @@ -2,7 +2,12 @@ * This file handles preparing babel config for Linaria preevaluation. */ -import { PluginItem, TransformOptions } from '@babel/core'; +import { + PluginItem, + TransformOptions, + PluginTarget, + PluginOptions, +} from '@babel/core'; import { StrictOptions } from '../types'; type DefaultOptions = Partial & { @@ -11,6 +16,125 @@ type DefaultOptions = Partial & { caller: { evaluate: boolean }; }; +function getPluginTarget(item: PluginItem) { + const target = Array.isArray(item) ? item[0] : item; + try { + if (typeof target === 'string') { + return require.resolve(target); + } + return target; + } catch { + return target; + } +} + +function getPluginInstanceName(item: PluginItem) { + return Array.isArray(item) ? item[2] : undefined; +} + +function shouldMergePlugins(left: PluginItem, right: PluginItem) { + const leftTarget = getPluginTarget(left); + const rightTarget = getPluginTarget(right); + const leftName = getPluginInstanceName(left); + const rightName = getPluginInstanceName(right); + const result = leftTarget === rightTarget && leftName === rightName; + return result; +} + +function isMergablePlugin( + item: PluginItem +): item is + | PluginTarget + | [PluginTarget, PluginOptions] + | [PluginTarget, PluginOptions, string | undefined] { + return typeof item === 'string' || Array.isArray(item); +} + +function getPluginOptions(item: PluginItem): object { + if (typeof item === 'string') { + return {}; + } + + if (Array.isArray(item)) { + return { + ...item[1], + }; + } + + if (typeof item === 'object' && 'options' in item) { + return { + ...item.options, + }; + } + + return {}; +} + +// Merge two plugin declarations. Options from the right override options +// on the left +function mergePluginItems(left: PluginItem, right: PluginItem) { + if (isMergablePlugin(left) && isMergablePlugin(right)) { + const pluginTarget = Array.isArray(left) ? left[0] : left; + const leftOptions = getPluginOptions(left); + const rightOptions = getPluginOptions(right); + return [pluginTarget, { ...leftOptions, ...rightOptions }]; + } + + return right; +} + +/** + * Add `newItem` to the beginning of `plugins`. If a plugin with the same + * target (and ID) already exists, it will be dropeed, and its options + * merged into those of the newly added item. + */ +export function mergeOrPrependPlugin( + plugins: PluginItem[], + newItem: PluginItem +) { + const mergeItemIndex = plugins.findIndex(existingItem => + shouldMergePlugins(existingItem, newItem) + ); + if (mergeItemIndex === -1) { + return [newItem, ...plugins]; + } + + const mergedItem = mergePluginItems(plugins[mergeItemIndex], newItem); + return [ + mergedItem, + ...plugins.slice(0, mergeItemIndex), + ...plugins.slice(mergeItemIndex + 1), + ]; +} + +/** + * Add `newItem` to the end of `plugins`. If an item with the same target (and + * ID) already exists, instead update its options from `newItem`. + */ +export function mergeOrAppendPlugin( + plugins: PluginItem[], + newItem: PluginItem +) { + const mergeItemIndex = plugins.findIndex(existingItem => + shouldMergePlugins(existingItem, newItem) + ); + if (mergeItemIndex === -1) { + return [...plugins, newItem]; + } + + const mergedItem = mergePluginItems(plugins[mergeItemIndex], newItem); + return [ + ...plugins.slice(0, mergeItemIndex), + ...plugins.slice(mergeItemIndex + 1), + mergedItem, + ]; +} + +function isLinariaBabelPreset(item: PluginItem) { + const name = getPluginTarget(item); + return name === 'linaria/babel' || name === require.resolve('../../babel'); +} + export default function buildOptions( filename: string, options?: StrictOptions @@ -47,56 +171,33 @@ export default function buildOptions( // If we programmatically pass babel options while there is a .babelrc, babel might throw // We need to filter out duplicate presets and plugins so that this doesn't happen // This workaround isn't full proof, but it's still better than nothing - const keys: Array = [ - 'presets', - 'plugins', - ]; - keys.forEach(field => { - babelOptions[field] = babelOptions[field] - ? babelOptions[field]!.filter((item: PluginItem) => { - // If item is an array it's a preset/plugin with options ([preset, options]) - // Get the first item to get the preset.plugin name - // Otherwise it's a plugin name (can be a function too) - const name = Array.isArray(item) ? item[0] : item; - - if ( - // In our case, a preset might also be referring to linaria/babel - // We require the file from internal path which is not the same one that we export - // This case won't get caught and the preset won't filtered, even if they are same - // So we add an extra check for top level linaria/babel - name === 'linaria/babel' || - name === require.resolve('../../babel') || - // Also add a check for the plugin names we include for bundler support - plugins.includes(name) - ) { - return false; - } - - // Loop through the default presets/plugins to see if it already exists - return !defaults[field].some(it => - // The default presets/plugins can also have nested arrays, - Array.isArray(it) ? it[0] === name : it === name - ); - }) - : []; - }); + // + // In our case, a preset might also be referring to linaria/babel + // We require the file from internal path which is not the same one that we export + // This case won't get caught and the preset won't filtered, even if they are same + // So we add an extra check for top level linaria/babel + let configuredPresets = (babelOptions.presets || []).filter( + item => !isLinariaBabelPreset(item) + ); + for (const requiredPreset of defaults.presets) { + // Preset order is last to first, so add our required presets to the end + // This makes sure that our preset is always run first + configuredPresets = mergeOrAppendPlugin(configuredPresets, requiredPreset); + } + + let configuedPlugins = babelOptions.plugins || []; + for (const requiredPlugin of defaults.plugins.slice().reverse()) { + // Plugin order is first to last, so add our required plugins to the start + // This makes sure that the plugins we specify always run first + configuedPlugins = mergeOrPrependPlugin(plugins, requiredPlugin); + } return { // Passed options shouldn't be able to override the options we pass // Linaria's plugins rely on these (such as filename to generate consistent hash) ...babelOptions, ...defaults, - presets: [ - // Preset order is last to first, so add the extra presets to start - // This makes sure that our preset is always run first - ...babelOptions.presets!, - ...defaults.presets, - ], - plugins: [ - ...defaults.plugins, - // Plugin order is first to last, so add the extra presets to end - // This makes sure that the plugins we specify always run first - ...babelOptions.plugins!, - ], + presets: configuredPresets, + plugins: configuedPlugins, }; } From a2329e4d2a0494403d638d7c9d5a2eddb8422de7 Mon Sep 17 00:00:00 2001 From: Jake Teton-Landis Date: Mon, 18 May 2020 11:14:47 -0700 Subject: [PATCH 2/6] shaker: use new babel options merging --- src/babel/evaluators/shaker/index.ts | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/babel/evaluators/shaker/index.ts b/src/babel/evaluators/shaker/index.ts index d55bd3e3e..8c161896c 100644 --- a/src/babel/evaluators/shaker/index.ts +++ b/src/babel/evaluators/shaker/index.ts @@ -3,7 +3,7 @@ import { debug } from '../../utils/logger'; import generator from '@babel/generator'; import { Evaluator, StrictOptions } from '../../types'; import { transformSync, types } from '@babel/core'; -import buildOptions from '../buildOptions'; +import buildOptions, { mergeOrPrependPlugin } from '../buildOptions'; function prepareForShake( filename: string, @@ -13,7 +13,8 @@ function prepareForShake( const transformOptions = buildOptions(filename, options); transformOptions.ast = true; - transformOptions.presets!.unshift([ + + transformOptions.presets = mergeOrPrependPlugin(transformOptions.presets!, [ '@babel/preset-env', { targets: 'ie 11', @@ -21,9 +22,16 @@ function prepareForShake( include: ['@babel/plugin-transform-template-literals'], }, ]); - transformOptions.presets!.unshift([require.resolve('../preeval'), options]); - transformOptions.plugins!.unshift('transform-react-remove-prop-types'); - transformOptions.plugins!.unshift([ + transformOptions.presets = mergeOrPrependPlugin(transformOptions.presets!, [ + require.resolve('../preeval'), + options, + ]); + + transformOptions.plugins = mergeOrPrependPlugin( + transformOptions.plugins!, + 'transform-react-remove-prop-types' + ); + transformOptions.plugins = mergeOrPrependPlugin(transformOptions.plugins!, [ '@babel/plugin-transform-runtime', { useESModules: false }, ]); From 5a42c1a196529f528e6acfb8a48b98e04e1841c3 Mon Sep 17 00:00:00 2001 From: Jake Teton-Landis Date: Mon, 18 May 2020 11:18:46 -0700 Subject: [PATCH 3/6] extractor: use new babel options merging --- src/babel/evaluators/extractor/index.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/babel/evaluators/extractor/index.ts b/src/babel/evaluators/extractor/index.ts index 86e1f0314..6e63ac4b6 100644 --- a/src/babel/evaluators/extractor/index.ts +++ b/src/babel/evaluators/extractor/index.ts @@ -9,7 +9,7 @@ import traverse, { NodePath } from '@babel/traverse'; import generator from '@babel/generator'; import { Evaluator } from '../../types'; -import buildOptions from '../buildOptions'; +import buildOptions, { mergeOrPrependPlugin } from '../buildOptions'; import RequirementsResolver from './RequirementsResolver'; // Checks that passed node is `exports.__linariaPreval = /* something */` @@ -45,8 +45,11 @@ function isLinariaPrevalExport( const extractor: Evaluator = (filename, options, text, only = null) => { const transformOptions = buildOptions(filename, options); - transformOptions.presets!.unshift([require.resolve('../preeval'), options]); - transformOptions.plugins!.unshift([ + transformOptions.presets = mergeOrPrependPlugin(transformOptions.presets!, [ + require.resolve('../preeval'), + options, + ]); + transformOptions.plugins = mergeOrPrependPlugin(transformOptions.plugins!, [ '@babel/plugin-transform-runtime', { useESModules: false }, ]); @@ -59,8 +62,7 @@ const extractor: Evaluator = (filename, options, text, only = null) => { // Then Linaria can identify all `styled` as call expressions, including `styled.h1`, `styled.p` and others. // Presets ordering is from last to first, so we add the plugin at the beginning of the list, which persist the order that was established with formerly used `@babel/preset-env`. - - transformOptions.presets!.unshift({ + transformOptions.presets = mergeOrPrependPlugin(transformOptions.presets!, { plugins: ['@babel/plugin-transform-template-literals'], }); // Expressions will be extracted only for __linariaPreval. From 1fa96ccfd2f76715685ecf9d2a9b0f08e4eeb6ce Mon Sep 17 00:00:00 2001 From: Jake Teton-Landis Date: Sat, 23 May 2020 12:58:59 -0700 Subject: [PATCH 4/6] fix "yarn watch --build" --- package.json | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index fedda27c1..b144b7ff0 100644 --- a/package.json +++ b/package.json @@ -43,7 +43,7 @@ "build:esm": "babel src --out-dir esm --extensions '.js,.jsx,.ts,.tsx' --ignore '**/__tests__/**,**/__integration-tests__/**,**/__fixtures__/**' --source-maps --delete-dir-on-start", "build": "yarn build:lib && yarn build:esm", "build:declarations": "tsc --emitDeclarationOnly --outDir lib", - "watch": "yarn build --watch", + "watch": "yarn build:lib --watch", "prepare": "yarn build && yarn build:declarations", "release": "release-it", "website": "yarn --cwd website", @@ -143,4 +143,5 @@ "/jest/resolveTypescript.js" ] } -} \ No newline at end of file +} + From 7b1ebc447e11d6004b634c56e9027ba8eccb1c9e Mon Sep 17 00:00:00 2001 From: Jake Teton-Landis Date: Sat, 23 May 2020 12:59:22 -0700 Subject: [PATCH 5/6] fix typo --- src/babel/evaluators/buildOptions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/babel/evaluators/buildOptions.ts b/src/babel/evaluators/buildOptions.ts index 73e74428e..8f919231a 100644 --- a/src/babel/evaluators/buildOptions.ts +++ b/src/babel/evaluators/buildOptions.ts @@ -189,7 +189,7 @@ export default function buildOptions( for (const requiredPlugin of defaults.plugins.slice().reverse()) { // Plugin order is first to last, so add our required plugins to the start // This makes sure that the plugins we specify always run first - configuedPlugins = mergeOrPrependPlugin(plugins, requiredPlugin); + configuedPlugins = mergeOrPrependPlugin(configuedPlugins, requiredPlugin); } return { From ba41fb8c389f12cf337bf8f07052e083c4473ea0 Mon Sep 17 00:00:00 2001 From: Jake Teton-Landis Date: Sat, 23 May 2020 13:14:29 -0700 Subject: [PATCH 6/6] longer timeout on flakey test --- src/__tests__/detect-core-js.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/__tests__/detect-core-js.test.js b/src/__tests__/detect-core-js.test.js index 10f7d1c33..ce2db78cf 100644 --- a/src/__tests__/detect-core-js.test.js +++ b/src/__tests__/detect-core-js.test.js @@ -29,4 +29,4 @@ it('Ensures that package do not include core-js dependency after build', async ( expect(result).toContain( 'Based on your code and targets, core-js polyfills were not added' ); -}); +}, 15000);