fix(test runner): allow directory imports with path mapping #32491
Conversation
This comment has been minimized.
This comment has been minimized.
dgozman
force-pushed
the
tr-fix-tsconfig-paths
branch
from
f2b9a33 to
f555d90
Compare
This comment has been minimized.
This comment has been minimized.
| @@ -367,8 +367,8 @@ export const test = base | |||
| const TSCONFIG = { | |||
| 'compilerOptions': { | |||
| 'target': 'ESNext', | |||
| 'moduleResolution': 'node', | |||
| 'module': 'commonjs', | |||
| 'moduleResolution': 'bundler', | |||
There was a problem hiding this comment.
This updates our default tsc config in tests to align with what we aim to support, particularly "bundler". Let me land it separately.
packages/playwright/src/util.ts
Outdated
| // - https://nodejs.org/dist/latest-v20.x/docs/api/modules.html#folders-as-modules | ||
|
|
||
| const mainField = getMainFieldFromPackageJson(packageJsonPath); | ||
| const mainFieldResolved = mainField ? resolveImportSpecifierExtension(path.resolve(resolved, mainField), isPathMapping) : undefined; |
There was a problem hiding this comment.
Why do we resolve package's main via path mapping? I don't think we should respect aliases in package.json, so should we just defer to Node from here?
There was a problem hiding this comment.
We do not apply path mappings here. resolveImportSpecifierExtension handles extension substitution, similarly to typescript.
Deferring to Node does not work, that's exactly the issue we are fixing. TypeScript allows ESM directory imports, while pure Node does not.
dgozman
force-pushed
the
tr-fix-tsconfig-paths
branch
from
f555d90 to
00b37ab
Compare
This comment has been minimized.
This comment has been minimized.
This regressed in microsoft#32078.
dgozman
force-pushed
the
tr-fix-tsconfig-paths
branch
from
00b37ab to
52589e8
Compare
Test results for "tests 1"1 failed 2 flaky29635 passed, 638 skipped Merge workflow run. |
…ts with path mapping We now hopefully align with `moduleResolution: bundler` tsconfig option, allowing directory imports in every scenario, and allowing proper module imports when not going through the type mapping. This regressed in microsoft#32078. Fixes microsoft#32480, fixes microsoft#31811.
We now hopefully align with
moduleResolution: bundlertsconfig option, allowing directory imports in every scenario, and allowing proper module imports when not going through the type mapping.This regressed in #32078. Fixes #32480, fixes #31811.