fix(typescript): allow directory imports

[kristojorg]

This updates previous work in #22887 to align more fully with --moduleResolution=bundler, allowing index files to be imported with the /index extension

[kristojorg]

@microsoft-github-policy-service agree

[kristojorg]

@dgozman Good suggestions, thanks. I've updated

[kristojorg]

not totally sure what happened to these checks, maybe we should re run them?

[kristojorg]

@dgozman, wondering if there is something I can do to unblock this? The test failures look unrelated to the work I did

[dgozman]

@dgozman, wondering if there is something I can do to unblock this? The test failures look unrelated to the work I did

Sorry for delay, holidays here. The failures are indeed related - fs.lstatSync() throws, see my comments.

[kristojorg]

The failures are indeed related - fs.lstatSync() throws, see my comments.

I've added a try/catch and replaced lstatSync with statSync.

[kristojorg]

@dgozman I'm just struggling to understand the error the failing test is pointing at. It appears to me that the expect block is being incorrectly interpreted as an array of strings when in fact it is an array of regexs.

I also can't get it to fail locally

[dgozman]

This looks very good. Let's avoid the try/catch (see below), and should be good to go.

I've added a try/catch

Why don't we pass throwIfNoEntry: false option to avoid exceptions altogether? This should be much faster.

I'm just struggling to understand the error the failing test is pointing at.

This seems unrelated to your change, we'll take a look on our own. Sorry for the inconvenience.

[kristojorg]

Why don't we pass throwIfNoEntry: false option to avoid exceptions altogether? This should be much faster.

I tried this actually and Typescript was giving me an error that it was an invalid option. I figured it must have been added in a newer version of Node than we had installed, but it appears to just be missing from the 14.x types. I've added it with a comment and ts-ignore. Lmk if you'd like me to change that

[mxschmitt]

Why don't we pass throwIfNoEntry: false option to avoid exceptions altogether? This should be much faster.

I tried this actually and Typescript was giving me an error that it was an invalid option. I figured it must have been added in a newer version of Node than we had installed, but it appears to just be missing from the 14.x types. I've added it with a comment and ts-ignore. Lmk if you'd like me to change that

ts-ignore is unfortunately not an option for us. Let's upgrade @types/node to 16.x since we dropped support for Node.js 14? (as a separate PR). If you don't want to do it, I can do it later. Thanks!

[kristojorg]

Made a PR here: https://github.com/microsoft/playwright/pull/23429

[github-actions[bot]]

"tests 1" report.

[github-actions[bot]]

"tests 1" report.

[kristojorg]

Hey there, I'm wondering if you know when this will be released?

[mxschmitt]

This should be already included in v1.35.0.

[kristojorg]

I see, yes it is in that release. It seems this isn't working for directory imports from node_modules. For example if I have import pkg from "@org/package", I get a ENOENT: no such file or directory error. The path it is trying to stat however, is /my/source/code/@org/package. I have two questions:

1. Is there a different way to handle node_module imports?
2. Should I make this function throw a better error if neither a file or directory exists at the given location, even with the various extensions and index we try adding?

[mxschmitt]

Can you file an issue and we can continue there? Easier to track, thanks!