-
Notifications
You must be signed in to change notification settings - Fork 8
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
refactor(arui-scripts): refactoring linter rules #169
Changes from all commits
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 |
---|---|---|
|
@@ -34,8 +34,7 @@ describe('update-with-presets', () => { | |
overridesPath: ['package-overrides-path.js'], | ||
} as AppContext; | ||
|
||
// eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
const updatedConfig = updateWithPresets(baseConfig, context); | ||
updateWithPresets(baseConfig, context); | ||
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. а куда переменная |
||
|
||
expect(context.overridesPath).toEqual([ | ||
'presets/arui-scripts.overrides', | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,39 +1,38 @@ | ||
// TODO: remove eslint-disable-next-line | ||
import merge from 'lodash.merge'; | ||
|
||
import { tryResolve } from '../util/resolve'; | ||
|
||
import { AppConfigs, AppContext } from './types'; | ||
import { validateSettingsKeys } from './validate-settings-keys'; | ||
|
||
const ES_MODULE = '__esModule'; | ||
|
||
export function updateWithPresets(config: AppConfigs, context: AppContext) { | ||
if (!config.presets) { | ||
return config; | ||
} | ||
if (!config.presets) return config; | ||
|
||
const presetsConfigPath = tryResolve(`${config.presets}/arui-scripts.config`, { | ||
let appConfig = config; | ||
const presetsConfigPath = tryResolve(`${appConfig.presets}/arui-scripts.config`, { | ||
paths: [context.cwd], | ||
}); | ||
const presetsOverridesPath = tryResolve(`${config.presets}/arui-scripts.overrides`, { | ||
const presetsOverridesPath = tryResolve(`${appConfig.presets}/arui-scripts.overrides`, { | ||
paths: [context.cwd], | ||
}); | ||
|
||
if (presetsConfigPath) { | ||
// eslint-disable-next-line import/no-dynamic-require, @typescript-eslint/no-var-requires, global-require | ||
let presetsSettings = require(presetsConfigPath); | ||
|
||
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. require тут можно динамическим импортом заменить, тогда eslint комменты можно будет снести |
||
// eslint-disable-next-line no-underscore-dangle | ||
if (presetsSettings.__esModule) { | ||
if (ES_MODULE in presetsSettings) { | ||
// ts-node импортирует esModules, из них надо вытягивать default именно так | ||
presetsSettings = presetsSettings.default; | ||
} | ||
validateSettingsKeys(config, presetsSettings, presetsConfigPath); | ||
// eslint-disable-next-line no-param-reassign | ||
config = merge(config, presetsSettings); | ||
validateSettingsKeys(appConfig, presetsSettings, presetsConfigPath); | ||
|
||
appConfig = merge(appConfig, presetsSettings); | ||
} | ||
if (presetsOverridesPath) { | ||
context.overridesPath.unshift(presetsOverridesPath); | ||
} | ||
|
||
return config; | ||
} | ||
return appConfig; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,3 @@ | ||
// TODO: remove eslint-disable-next-line | ||
import type { Configuration as WebpackConfiguration, WebpackOptionsNormalized } from 'webpack'; | ||
import type { Configuration as WebpackDevServerConfiguration } from 'webpack-dev-server'; | ||
|
||
|
@@ -44,7 +43,7 @@ type BoundCreateSingleClientWebpackConfig = OmitFirstArg<typeof createSingleClie | |
type ClientWebpackAdditionalArgs = { | ||
createSingleClientWebpackConfig: BoundCreateSingleClientWebpackConfig; | ||
findLoader: typeof findLoader; | ||
findPlugin: ReturnType<typeof findPlugin<'client'>> | ||
findPlugin: ReturnType<typeof findPlugin<'client'>>; | ||
}; | ||
|
||
type ServerWebpackAdditionalArgs = { | ||
|
@@ -75,7 +74,7 @@ type OverrideFunction< | |
> = ( | ||
config: Overrides[K], | ||
appConfig: AppContextWithConfigs, | ||
additionalArgs: AdditionalArgs, | ||
additionalArgs: AdditionalArgs | unknown, | ||
) => Overrides[K]; | ||
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. не надо тут unknown. Обсуждали в чате - этот тип выставляется пользователям. Сейчас он точно говорит что за тип ожидать при использовании оверрайда. Пример использования на проекте const override: OverrideFile = {
webpackClient: (config, appConfig, { createSingleClientWebpackConfig }) => {
return createSingleClientWebpackConfig('src/index', 'something');
}
}; С типами без unknown - все работает. Если добавить unknown - ts будет ругаться. Нужно чтобы не ругался. Если при этом никак не обойтись без использования например any в коде библиотеки - это ок, правильность и удобство типов для потребителей важнее чем прохождение правил линтера в библиотеке. |
||
|
||
export type OverrideFile = { | ||
|
@@ -89,8 +88,7 @@ overrides = appConfigs.overridesPath.map((path) => { | |
// eslint-disable-next-line import/no-dynamic-require, global-require, @typescript-eslint/no-var-requires | ||
const requireResult = require(path); | ||
|
||
// eslint-disable-next-line no-underscore-dangle | ||
if (requireResult.__esModule) { | ||
if ('__esModule' in requireResult) { | ||
// ts-node импортирует esModules, из них надо вытягивать default именно так | ||
return requireResult.default; | ||
} | ||
|
@@ -111,32 +109,29 @@ overrides = appConfigs.overridesPath.map((path) => { | |
* @param args Дополнительные аргументы, которые будут переданы в функцию оверрайда | ||
* @returns {*} | ||
*/ | ||
function applyOverrides< | ||
T extends Overrides[Key], | ||
Key extends keyof Overrides, | ||
// eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
Args = Key extends keyof OverridesAdditionalArgs ? OverridesAdditionalArgs[Key] : undefined, | ||
>(overridesKey: Key | Key[], config: T, args?: any): T { | ||
if (typeof overridesKey === 'string') { | ||
// eslint-disable-next-line no-param-reassign | ||
overridesKey = [overridesKey]; | ||
} | ||
overridesKey.forEach((key) => { | ||
function applyOverrides<T extends Overrides[Key], Key extends keyof Overrides>( | ||
overridesKey: Key | Key[], | ||
config: T, | ||
args?: unknown, | ||
): T { | ||
let tempConfig = config; | ||
const overrideKeys = typeof overridesKey === 'string' ? [overridesKey] : overridesKey; | ||
|
||
overrideKeys.forEach((key) => { | ||
overrides.forEach((override) => { | ||
// eslint-disable-next-line no-prototype-builtins | ||
if (override.hasOwnProperty(key)) { | ||
if (key in override) { | ||
const overrideFn = override[key]; | ||
|
||
if (typeof overrideFn !== 'function') { | ||
throw new TypeError(`Override ${key} must be a function`); | ||
} | ||
// eslint-disable-next-line no-param-reassign | ||
config = overrideFn(config, appConfigs, args); | ||
|
||
tempConfig = overrideFn(tempConfig, appConfigs, args); | ||
} | ||
}); | ||
}); | ||
|
||
return config; | ||
return tempConfig; | ||
} | ||
|
||
export default applyOverrides; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,4 +27,4 @@ export function findLoader( | |
} | ||
|
||
return undefined; | ||
} | ||
} |
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.
здесь лучше сделать
import { sync } from 'brotli-size';
если такое возможно, тк есть мнение что так tree shaking лучше сработает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.
tree shaking в консольной утилите не то чтоб сильно актуален. Я за то чтоб оставить либо текущую запись, либо переделать на такую же, как и у gzip-size, потому что что такое функция sync - понять совсем не реально, а что такое brotlySize.sync - вполне