1898807 - "./mach puppeteer-test" fails due to conflict with "node_modules/@types" which symlinks to Gecko-only internal types

Status: RESOLVED FIXED

(Remote Protocol :: Agent, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Not sure when this started but this failure is re-appearing quite often and forces us to remove the whole node_modules folder from the local tree. Running ./mach puppeteer-test the following lines will be printed and the command aborts:

src/injected/Poller.ts(98,9): error TS2552: Cannot find name 'window'. Did you mean 'Window'?
src/injected/Poller.ts(104,5): error TS2552: Cannot find name 'window'. Did you mean 'Window'?
src/common/WaitTask.ts(127,26): error TS2552: Cannot find name 'document'. Did you mean 'Document'?
[..] // way more of these
src/common/BrowserConnector.ts(103,17): error TS2339: Property 'webSocketDebuggerUrl' does not exist on type 'JSON'.

Completed build with errors in 6s

❌ 1 script failed.

I was not able yet to figure out which module(s) exactly cause this problem.

Mark do you have an idea?

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I tried to reproduce again and as it looks like the issue starts to happen when some tests are run on their own, like:

1. Run ./mach puppeteer-test to ensure it works
2. Open navigation.spec.ts and change one it() to it.only().
3. Run ./mach puppeteer-test again and it fails
4. Reverting the code change the issue seems to be gone

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

It seems to work as long as I did not run the mach lint --outgoing --warning --fix command. Then it starts to fail.

I've now tried to remove modules individually and the culprit here seems to be the @types/gecko symlink:

lrwxr-xr-x 1 henrik staff 18 May 27 11:03 node_modules/@types/gecko -> ../../tools/@types

This points to:
https://searchfox.org/mozilla-central/source/tools/@types

Strangely, after running mach lint again it doesn't fail anymore for now. Not sure what else is needed to get into the same state again.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Oh wait. I actually removed all files under tools/@types which clearly explains it. After resetting the tree the failure is present again.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

When installing Puppeteer the following types are installed under node_modules/@types:

drwxr-xr-x   6 henrik  staff   192 29 Mai 09:24 argparse
drwxr-xr-x   9 henrik  staff   288 29 Mai 09:24 chrome
drwxr-xr-x   6 henrik  staff   192 29 Mai 09:24 debug
drwxr-xr-x   7 henrik  staff   224 29 Mai 09:24 diff
drwxr-xr-x   6 henrik  staff   192 29 Mai 09:24 doctrine
drwxr-xr-x   9 henrik  staff   288 29 Mai 09:24 eslint
drwxr-xr-x   7 henrik  staff   224 29 Mai 09:24 estree
drwxr-xr-x   6 henrik  staff   192 29 Mai 09:24 istanbul-lib-coverage
drwxr-xr-x   6 henrik  staff   192 29 Mai 09:24 istanbul-lib-report
drwxr-xr-x   6 henrik  staff   192 29 Mai 09:24 istanbul-reports
drwxr-xr-x   6 henrik  staff   192 29 Mai 09:24 json-schema
drwxr-xr-x   6 henrik  staff   192 29 Mai 09:24 json5
drwxr-xr-x   8 henrik  staff   256 29 Mai 09:24 mime
drwxr-xr-x   6 henrik  staff   192 29 Mai 09:24 minimist
drwxr-xr-x   6 henrik  staff   192 29 Mai 09:24 mocha
drwxr-xr-x   6 henrik  staff   192 29 Mai 09:24 ms
drwxr-xr-x  58 henrik  staff  1856 29 Mai 09:24 node
drwxr-xr-x   6 henrik  staff   192 29 Mai 09:24 normalize-package-data
drwxr-xr-x   6 henrik  staff   192 29 Mai 09:24 pixelmatch
drwxr-xr-x   7 henrik  staff   224 29 Mai 09:24 pngjs
drwxr-xr-x   6 henrik  staff   192 29 Mai 09:24 progress
drwxr-xr-x  11 henrik  staff   352 29 Mai 09:24 semver
drwxr-xr-x   6 henrik  staff   192 29 Mai 09:24 sinon
drwxr-xr-x   6 henrik  staff   192 29 Mai 09:24 sinonjs__fake-timers
drwxr-xr-x   7 henrik  staff   224 29 Mai 09:24 source-map-support
drwxr-xr-x   6 henrik  staff   192 29 Mai 09:24 stack-utils
drwxr-xr-x   6 henrik  staff   192 29 Mai 09:24 tar-fs
drwxr-xr-x   6 henrik  staff   192 29 Mai 09:24 tar-stream
drwxr-xr-x   6 henrik  staff   192 29 Mai 09:24 through
drwxr-xr-x   6 henrik  staff   192 29 Mai 09:24 unbzip2-stream
drwxr-xr-x   7 henrik  staff   224 29 Mai 09:24 ws
drwxr-xr-x  10 henrik  staff   320 29 Mai 09:24 yargs
drwxr-xr-x   6 henrik  staff   192 29 Mai 09:24 yargs-parser
drwxr-xr-x   6 henrik  staff   192 29 Mai 09:24 yauzl

Removing this folder or making it a symlink to tools/@types breaks linting for Puppeteer.

As it looks like bug 1880764 regressed this given that we do not keep all the subfolders under @types but add a soft-link to Gecko-only types.

Shouldn't we add this soft-link as node_modules/@types/gecko?

Comment 5

[Tomislav Jovanovic :zombie]

When installing Puppeteer the following types are installed under node_modules/@types:

How do I "install puppeteer" in a fresh checkout of m-c?

Removing this folder or making it a symlink to tools/@types breaks linting for Puppeteer.

As it looks like bug 1880764 regressed this given that we do not keep all the subfolders under @types but add a soft-link to Gecko-only types.

Shouldn't we add this soft-link as node_modules/@types/gecko?

I might be confused, but isn't that what I did in bug 1880764? Here what my <topsrcdir>/node_modules/@types/ looks like:

$ ls -la node_modules/@types
total 92
drwxr-xr-x 1 zombie None  0 Mar 14 21:11 .
drwxr-xr-x 1 zombie None  0 Apr 11 16:35 ..
lrwxrwxrwx 1 zombie None 32 Mar 14 21:11 gecko -> /d/mozilla/unified/tools/@types/
drwxr-xr-x 1 zombie None  0 Mar 14 21:11 json5
drwxr-xr-x 1 zombie None  0 Mar 14 21:11 json-schema
drwxr-xr-x 1 zombie None  0 Mar 14 21:11 linkify-it
drwxr-xr-x 1 zombie None  0 Mar 14 21:11 markdown-it
drwxr-xr-x 1 zombie None  0 Mar 14 21:11 mdurl
drwxr-xr-x 1 zombie None  0 Mar 14 21:11 minimist
drwxr-xr-x 1 zombie None  0 Mar 14 21:11 normalize-package-data
drwxr-xr-x 1 zombie None  0 Mar 14 21:11 semver

Are you seeing something else after running npm ci in your m-c?

In any case, I believe the solution here is to be explicit where types should come from, by specifying typeRoots like in bug 1891209, otherwise tsc is being "smart" and looking up everything it can find, which can lead to issues.

(or if your tsconfig.json is vendored, and you don't want to modify it, you can also pass in the --typeRoots cli argument to tsc like I'm doing in D209620.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Tomislav Jovanovic :zombie from comment #5)

When installing Puppeteer the following types are installed under node_modules/@types:

How do I "install puppeteer" in a fresh checkout of m-c?

Puppeteer is vendored in. So just run the steps as given in comment 1.

I might be confused, but isn't that what I did in bug 1880764? Here what my <topsrcdir>/node_modules/@types/ looks like:

$ ls -la node_modules/@types
total 92
drwxr-xr-x 1 zombie None  0 Mar 14 21:11 .
drwxr-xr-x 1 zombie None  0 Apr 11 16:35 ..
lrwxrwxrwx 1 zombie None 32 Mar 14 21:11 gecko -> /d/mozilla/unified/tools/@types/
drwxr-xr-x 1 zombie None  0 Mar 14 21:11 json5
drwxr-xr-x 1 zombie None  0 Mar 14 21:11 json-schema
drwxr-xr-x 1 zombie None  0 Mar 14 21:11 linkify-it
drwxr-xr-x 1 zombie None  0 Mar 14 21:11 markdown-it
drwxr-xr-x 1 zombie None  0 Mar 14 21:11 mdurl
drwxr-xr-x 1 zombie None  0 Mar 14 21:11 minimist
drwxr-xr-x 1 zombie None  0 Mar 14 21:11 normalize-package-data
drwxr-xr-x 1 zombie None  0 Mar 14 21:11 semver

Are you seeing something else after running npm ci in your m-c?

Yes, that's what I'm seeing as well. So please drop this specific question from my last comment. The problem here comes from tools/@types/lib.gecko.dom.d.ts.

In any case, I believe the solution here is to be explicit where types should come from, by specifying typeRoots like in bug 1891209, otherwise tsc is being "smart" and looking up everything it can find, which can lead to issues.

(or if your tsconfig.json is vendored, and you don't want to modify it, you can also pass in the --typeRoots cli argument to tsc like I'm doing in D209620.

So Puppeteer is vendored in https://searchfox.org/mozilla-central/source/remote/test/puppeteer/ and has a couple of tsconfig files. So not sure which is the one we actually use here. Maybe under packages/puppeter/tsconfig.json?

What I just noticed is that when installing Puppeteer in-tree the node_modules folder is created as /remote/test/puppeteer/node_modules but running ./mach puppeteer-test probably uses the node modules from /node-modules? Is there a way to generally force a specific node_modules folder?

Comment 7

[Tomislav Jovanovic :zombie]

Puppeteer is vendored in. So just run the steps as given in comment 1.

Do I need to run it from a subdir or is there some dependency missing? Because I get a few errors like

npm ERR! Lifecycle script `clean` failed with error:
npm ERR! Error: command failed

and then it runs for a while before throwing

UnicodeDecodeError: 'charmap' codec can't decode byte 0x9d in position 2: character maps to <undefined>

So Puppeteer is vendored in https://searchfox.org/mozilla-central/source/remote/test/puppeteer/ and has a couple of tsconfig files. So not sure which is the one we actually use here. Maybe under packages/puppeter/tsconfig.json?

looks like most of them import tsconfig.base.json:
https://searchfox.org/mozilla-central/source/remote/test/puppeteer/tsconfig.base.json

What I just noticed is that when installing Puppeteer in-tree the node_modules folder is created as /remote/test/puppeteer/node_modules but running ./mach puppeteer-test probably uses the node modules from /node-modules? Is there a way to generally force a specific node_modules folder?

Not sure in general when running node stuff, but for tsc, that's exactly what an explicit typesRoots in tsconfig.json does, just put a relative path to the node_modules you want it to use, something like:

"typeRoots": ["./node_modules/@types"]

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:andi, could you have a look please?

For more information, please visit BugBot documentation.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Tomislav Jovanovic :zombie from comment #7)

Not sure in general when running node stuff, but for tsc, that's exactly what an explicit typesRoots in tsconfig.json does, just put a relative path to the node_modules you want it to use, something like:

"typeRoots": ["./node_modules/@types"]

Adding that initially didn't help, but then I noticed that this entry needs to be under compilerOptions. Once added there in the tsconfig.base.json file, the issue was resolved. Thanks a lot for the help! I'm going to provide a patch and also submit an upstream PR to improve how the Puppeteer repository behaves when embedded in other repositories.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Bug 1898807 - [puppeteer] Fix build error for broken types in tsc:build.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8ae0bad1eed1
[puppeteer] Fix build error for broken types in tsc:build. r=webdriver-reviewers,alexrudenko,jdescottes DONTBUILD

Comment 12

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/8ae0bad1eed1

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:whimboo, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox128 to wontfix.

For more information, please visit BugBot documentation.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2]]

No, it's fine to have it fixed on mozilla-central.