core(fr): make FR navigations the default in node #13783
Conversation
build/build-bundle.js
Outdated
| @@ -91,6 +91,7 @@ async function build(entryPath, distPath, opts = {minify: true}) { | |||
| const shimsObj = {}; | |||
|
|
|||
| const modulesToIgnore = [ | |||
| 'puppeteer', | |||
There was a problem hiding this comment.
Puppeteer isn't needed if the environment provides a puppeteer page as a dependency. We don't need puppeteer bloating our bundle.
package.json
Outdated
| @@ -199,6 +198,7 @@ | |||
| "metaviewport-parser": "0.2.0", | |||
| "open": "^8.4.0", | |||
| "parse-cache-control": "1.0.1", | |||
| "puppeteer": "^10.2.0", | |||
There was a problem hiding this comment.
Since we do not need puppeteer's bundled chromium (we use chrome-launcher), we should use puppeteer-core instead.
https://github.com/puppeteer/puppeteer/blob/main/docs/api.md#puppeteer-vs-puppeteer-core
There was a problem hiding this comment.
In fact, it might be worth first doing a PR that changes our devDep from pup to pup-core.
There's also #13657 to consider.
There was a problem hiding this comment.
In that case, we can keep puppeteer as a dev dep because we use it in a few scripts, and then just have pup-core as the package dep.
…se into fr-as-default-node
| if (flags.fraggleRock) { | ||
| flags.channel = 'fraggle-rock-cli'; | ||
| } else { | ||
| flags.channel = 'cli'; |
There was a problem hiding this comment.
should we emit a console.warning stating that this is deprecated?
There was a problem hiding this comment.
Yeah but I think it makes more sense in the next PR, once we switch --fraggle-rock to --legacy-navigation. Right now the default behavior would be to emit the deprecation warning.
adamraine
changed the title
Doc
This PR makes FR navigations the default for the node API. One important requirement for this change is making
puppeteer-corea package dependency.