chore(agnostification): common/helper.ts #6515
Conversation
| if (!isNode && path) { | ||
| throw new Error('Cannot write to a path outside of Node.js environment.'); | ||
| } | ||
|
|
There was a problem hiding this comment.
Can't we do let fs; and then fs = ... inside one of the ifs here? (It seems fine to rely on Node.js' caching regardless, but if we can avoid it and simplify the code at the same time...)
There was a problem hiding this comment.
I meant let fs — updated comment to clarify
The `readProtocolStream` method uses `fs` only if you want to write to a
file. So we gate at the start of the function and ensure that if we got
given a path we are not in a Node environment.
We then use `import('fs')` to lazily load fs. We have multiple calls but
they will be cached so I don't believe that it's going to be a perf hit.
jackfranklin
force-pushed
the
helper-agnostic
branch
from
ded9c0f
to
fe8f6cf
Compare
| @@ -59,6 +59,8 @@ jobs: | |||
|
|
|||
| - node_js: "12.16.3" | |||
| name: 'Browser tests: Linux/Chromium' | |||
| addons: | |||
| chrome: stable | |||
There was a problem hiding this comment.
needed to ensure Travis has an up to date chrome that web test runner can use
There was a problem hiding this comment.
We could also point it to the bundled chromium which puppeteer already brings. Let me know if I can help here :)
There was a problem hiding this comment.
@LarsDenBakker thanks! I'm quite keen to see if it's stable on a regular Chrome, but if not we'll reach for that and I expect I'll ping you for help :D
| @@ -74,7 +74,7 @@ | |||
| "@types/ws": "^7.2.4", | |||
| "@typescript-eslint/eslint-plugin": "^4.4.0", | |||
| "@typescript-eslint/parser": "^4.4.0", | |||
| "@web/test-runner": "^0.8.4", | |||
| "@web/test-runner": "^0.9.2", | |||
There was a problem hiding this comment.
the old version started timing out on CI...upgrading to this fixes it. Not entirely sure why!
The
readProtocolStreammethod usesfsonly if you want to write to afile. So we gate at the start of the function and ensure that if we got
given a path we are not in a Node environment.
We then use
import('fs')to lazily load fs. We have multiple calls butthey will be cached so I don't believe that it's going to be a perf hit.