Skip to content
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

fix: normalize windows backslash for configFile and source #48

Merged
merged 8 commits into from
Dec 29, 2023

Conversation

cawa-93
Copy link
Contributor

@cawa-93 cawa-93 commented Nov 15, 2022

fix #47

This fix issue for me on Windows 10, but I can't test it on other platforms

src/loader.ts Outdated
@@ -218,7 +218,7 @@ async function resolveConfig (source: string, options: LoadConfigOptions): Promi
if (isDir) { source = options.configFile; }
const res: ResolvedConfig = { config: undefined, cwd };
try {
res.configFile = options.jiti.resolve(resolve(cwd, source), { paths: [cwd] });
res.configFile = resolve(options.jiti.resolve(resolve(cwd, source), { paths: [cwd] }));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please check result of jiti.resolve()? It is strange if it doesn't normalize windows path as is using pathe itself (https://github.com/unjs/jiti/blob/main/src/jiti.ts)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason it doesn't normalize path in all cases

{                                                                                                     
  cwd: 'C:/Users/kozac/AppData/Local/Temp/c12/github_unjs_c12/test/fixture',                          
  source: 'config',                                                                                   
  'resolve(cwd, source)': 'C:/Users/kozac/AppData/Local/Temp/c12/github_unjs_c12/test/fixture/config',
  'options.jiti.resolve': 'C:\\Users\\kozac\\AppData\\Local\\Temp\\c12\\github_unjs_c12\\test\\fixture\\config.ts',
  'resolve(options.jiti.resolve)': 'C:/Users/kozac/AppData/Local/Temp/c12/github_unjs_c12/test/fixture/config.ts'
}
{
  cwd: 'C:/Users/kozac/AppData/Local/Temp/c12/github_unjs_c12/test/fixture',
  source: './config.dev',
  'resolve(cwd, source)': 'C:/Users/kozac/AppData/Local/Temp/c12/github_unjs_c12/test/fixture/config.dev',
  'options.jiti.resolve': 'C:\\Users\\kozac\\AppData\\Local\\Temp\\c12\\github_unjs_c12\\test\\fixture\\config.dev.ts',
  'resolve(options.jiti.resolve)': 'C:/Users/kozac/AppData/Local/Temp/c12/github_unjs_c12/test/fixture/config.dev.ts'
}
{
  cwd: 'C:/Users/kozac/AppData/Local/Temp/c12/github_unjs_c12/test/fixture/node_modules/c12-npm-test',
  source: 'C:\\Users\\kozac\\AppData\\Local\\Temp\\c12\\github_unjs_c12\\test\\fixture\\node_modules\\c12-npm-test\\config.ts',
  'resolve(cwd, source)': 'C:/Users/kozac/AppData/Local/Temp/c12/github_unjs_c12/test/fixture/node_modules/c12-npm-test/config.ts',
  'options.jiti.resolve': 'C:/Users/kozac/AppData/Local/Temp/c12/github_unjs_c12/test/fixture/node_modules/c12-npm-test/config.ts',
  'resolve(options.jiti.resolve)': 'C:/Users/kozac/AppData/Local/Temp/c12/github_unjs_c12/test/fixture/node_modules/c12-npm-test/config.ts'
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Difference in thehe cases in extensions. 1-2 can't be resolved by resolvePathSync ( TypeError [ERR_INVALID_URL_SCHEME]: The URL must be of scheme file) and resolving by nativeRequire.resolve.
But last one resolving by resolvePathSync from mlly.

const jiti = require('.')(undefined, {
    esmResolve: true
})

console.log({
    'C:/Users/kozac/AppData/Local/Temp/c12/github_unjs_c12/test/fixture/config.ts': JSON.stringify(jiti.resolve('C:/Users/kozac/AppData/Local/Temp/c12/github_unjs_c12/test/fixture/config.ts', { paths: [cwd] })),
    'C:/Users/kozac/AppData/Local/Temp/c12/github_unjs_c12/test/fixture/config': JSON.stringify(jiti.resolve('C:/Users/kozac/AppData/Local/Temp/c12/github_unjs_c12/test/fixture/config', { paths: [cwd] })),
})
{
  'C:/Users/kozac/AppData/Local/Temp/c12/github_unjs_c12/test/fixture/config.ts': '"C:/Users/kozac/AppData/Local/Temp/c12/github_unjs_c12/test/fixture/config.ts"',
  'C:/Users/kozac/AppData/Local/Temp/c12/github_unjs_c12/test/fixture/config': '"C:\\\\Users\\\\kozac\\\\AppData\\\\Local\\\\Temp\\\\c12\\\\github_unjs_c12\\\\test\\\\fixture\\\\config.ts"'
}

@pi0 pi0 changed the title test: Fix windows paths fix: always normalize paths Feb 20, 2023
@codecov
Copy link

codecov bot commented Feb 20, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (7cd6095) 73.12% compared to head (1908fc3) 76.05%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #48      +/-   ##
==========================================
+ Coverage   73.12%   76.05%   +2.92%     
==========================================
  Files           7        7              
  Lines         774      781       +7     
  Branches       67       75       +8     
==========================================
+ Hits          566      594      +28     
+ Misses        206      185      -21     
  Partials        2        2              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pi0 pi0 changed the title fix: always normalize paths fix: normalize windows backslash for configFile and source Dec 29, 2023
@pi0 pi0 merged commit b78e4ee into unjs:main Dec 29, 2023
@pi0
Copy link
Member

pi0 commented Dec 29, 2023

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failed test after clean install on windows
2 participants