fix: ensure lint text does not strip rules between runs#802
Conversation
7fd294f to
80c3244
Compare
|
Yesterday, I was migrating And, after spending an absurd amount of time debugging, I realised that Lines 213 to 216 in 27bf5fb Here's a minimal reproducible example that highlights the issue well: // xo.config.js
const xoConfig = [
{
rules: {
'@typescript-eslint/no-empty-object-type': 'off',
'@stylistic/function-paren-newline': 'off',
},
},
];
export default xoConfig;async function main() {
const XO = (await import('xo')).default;
const xo = new XO({ cwd: 'some/path' });
await xo.lintText('', { filePath: 'some/path' });
console.log(xo.xoConfig);
}
main().then(() => main()); // Call `main` twice The output for the first [
{},
{
rules: { '@typescript-eslint/no-empty-object-type': 'off' },
files: ['**/*.{ts,tsx,cts,mts}'],
},
{
rules: { '@stylistic/function-paren-newline': 'off' },
files: ['**/*.{js,jsx,mjs,cjs,ts,tsx,cts,mts}'],
},
];However, during the split, the original config gets mutated:
So, when the second instance of Lines 163 to 165 in 27bf5fb Hence, the second [
{},
{
rules: { '@stylistic/function-paren-newline': 'off' },
files: ['**/*.{js,jsx,mjs,cjs,ts,tsx,cts,mts}'],
},
];So, here's a fix I came up with: - // Set the other rules to the original config
- config.rules = Object.fromEntries(otherRules);
- // These rules should still apply to all files
- config.files = [allFilesGlob];
- return [tsConfig, config];
+ const otherConfig: XoConfigItem = {
+ ...config,
+ // Set the other rules to the original config
+ rules: Object.fromEntries(otherRules),
+ };
+ // These rules should still apply to all files
+ otherConfig.files = [allFilesGlob];
+ return [tsConfig, otherConfig];It simply avoids mutating the original config, and this also seems to pass all the newly added tests. Wish I read seen this PR before delving into this 😅, but anyways, thought I'd share my debugging here and it might add some value. |
|
@som-sm Thanks much for digging in! Funny enough this wasn't happening in the pre-release, something changed when the deps were updated for this v1 release but its not clear to me what would have caused it. |
lintTextmultiple times, where some ts rules would get stripped after the first lint run. We now clone the config before each run.containsinstead ofisMatchfrom micromatch. This fixes matching on 'node_modules' which is common on excludes. I thinkisMatchmay still be correct forfilesandinclude.filesfrom tsconfigs during test. This doesn't change anything for tests.lint-textis more reliable for ts files when the file is written to disk, which we now do. This fixed some issues of getting different results between ubuntu and mac.find-cache-directoryfor ts files --stdin caching