Add --node-compat to Pages publish #2541

WalshyDev · 2023-01-12T23:59:11Z

What this PR solves / how to test:

This adds --node-compat support to Pages, this is a common feature request and will allow for the usage of Stripe and other such libs within Functions.

Associated docs issues/PR:
N/A

Author has included the following, where applicable:

Tests
Changeset

Reviewer has performed the following, where applicable:

Checked for inclusion of relevant tests
Checked for inclusion of a relevant changeset
Checked for creation of associated docs updates
Manually pulled down the changes and spot-tested

Fixes #1074

changeset-bot · 2023-01-12T23:59:14Z

🦋 Changeset detected

Latest commit: 2bc6376

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
wrangler	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

GregBrimble · 2023-01-13T00:00:07Z

#2540 (comment)

github-actions · 2023-01-13T00:01:12Z

A wrangler prerelease is available for testing. You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/6711485115/npm-package-wrangler-2541

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/6711485115/npm-package-wrangler-2541

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/6711485115/npm-package-wrangler-2541 dev path/to/script.js

Additional artifacts:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/6711485115/npm-package-cloudflare-pages-shared-2541

Note that these links will no longer work once the GitHub Actions artifact expires.

wrangler@3.15.0 includes the following runtime dependencies:

Package	Constraint	Resolved
`miniflare`	3.20231025.0	3.20231025.0
`workerd`	1.20231025.0	1.20231025.0
`workerd --version`	1.20231025.0	2023-10-25

|

Please ensure constraints are pinned, and miniflare/workerd minor versions match.

WalshyDev · 2023-01-13T00:05:22Z

I'd like to hold on this until we see where this new nodejs_compat compatibility flag goes, especially since we don't have an in-dash option for this at the moment. I think we're going to quite quickly be at a point where using this polyfill is more harmful than good.

The nodejs compat flag won't have parity for quite a while. In-dash option is also not a blocker, we've done this many times and it takes no time at all to this to the UI.
Considering we already support this in dev and build, I see no reason to not just add it here. People have wanted it ever since Functions was made. This brings us closer in parity with Workers and for basically 0 cost.

This also means the migration to the new flag is also the same rather than being another thing different.

codecov · 2023-01-13T00:09:17Z

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (4b07385) 75.34% compared to head (bb7f385) 72.97%.
Report is 538 commits behind head on main.

❗ Current head bb7f385 differs from pull request most recent head 2bc6376. Consider uploading reports for the commit 2bc6376 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2541      +/-   ##
==========================================
- Coverage   75.34%   72.97%   -2.37%     
==========================================
  Files         223      156      -67     
  Lines       12341     9748    -2593     
  Branches     3190     2570     -620     
==========================================
- Hits         9298     7114    -2184     
+ Misses       3043     2634     -409

Files	Coverage Δ
packages/wrangler/src/pages/build.ts	`85.48% <ø> (+15.36%)`	⬆️
packages/wrangler/src/pages/dev.ts	`18.94% <ø> (+2.73%)`	⬆️
packages/wrangler/src/pages/publish.tsx	`65.38% <50.00%> (ø)`

... and 188 files with indirect coverage changes

JacobMGEvans

Looks great, would like to see an integration test for this before it goes in, probably should go here: https://github.com/cloudflare/wrangler2/blob/main/packages/wrangler/src/__tests__/pages.test.ts 😃

JacobMGEvans · 2023-01-13T15:21:49Z

packages/wrangler/src/pages/publish.tsx

+		if (nodeCompat) {
+			console.warn(
+				"Enabling node.js compatibility mode for builtins and globals. This is experimental and has serious tradeoffs. Please see https://github.com/ionic-team/rollup-plugin-node-polyfills/ for more details."
+			);
+		}


Could you add a test that runs wrangler pages publish --node-compat

JacobMGEvans · 2023-01-13T16:41:15Z

I'd like to hold on this until we see where this new nodejs_compat compatibility flag goes, especially since we don't have an in-dash option for this at the moment. I think we're going to quite quickly be at a point where using this polyfill is more harmful than good.

The nodejs compat flag won't have parity for quite a while. In-dash option is also not a blocker, we've done this many times and it takes no time at all to this to the UI. Considering we already support this in dev and build, I see no reason to not just add it here. People have wanted it ever since Functions was made. This brings us closer in parity with Workers and for basically 0 cost.

This also means the migration to the new flag is also the same rather than being another thing different.

I agree with this take on the polyfill --node-compat the runtime compat flag won't directly interfere with this implementation, they just cannot be used together.

notwedtm · 2023-02-20T20:26:08Z

Any updates on when this might get merged in?

jadbox · 2023-09-29T13:48:53Z

I'm needing to bump this too.

I'm blocked from going into production with CF pages due to this [#2541] bug. This prevents me from using PG tcp pooling in my Page workers.

The other issue is that I'm unable to test against remote data with Pages workers due to this bug:
#3505

GregBrimble

Blocking until we've had internal product roadmap discussions.

stale

GregBrimble · 2023-10-31T20:04:17Z

Rather than accepting this as a arg on deploy, should we instead look at the config of the project in the dashboard and use that so it's all kept in one place? Sort of like how we did D1 when it was in beta and needed the shims?

WalshyDev · 2023-10-31T20:41:55Z

It's already an arg for Workers, if we did move it we'd also need to change dev behaviour to pull from the project which just diverges us more from Workers.

GregBrimble · 2023-10-31T21:19:35Z

But we don't yet have it for many other similar props like compat date and flags. Feels like we should solve syncing config separately to adding this node compat feature (so should stay consistent until we've got that bit figured out in a separate effort).

janpio · 2023-12-09T14:57:07Z

I can confirm that using the prerelease version mentioned above indeed let's me deploy a project using pg to GitHub Pages that I could not deploy before, as the build was running into weird errors messages (because nodejs_compat just was not the right solution for my problem with pg). ~~Unfortunately the deployment does not seem to work though, requests get a 522 and the deployment shows "Failed" in the web UI.~~

~~@WalshyDev Is this supposed to work on the deployed side as well or is this PR here only one side of the coin and there would need to be a change on the platform as well?~~

Update: Turned out to be the size limit of the worker (>1MB, I was testing on a free account) that just leads to a failed deployment, instead of the normal output you get with Cloudflare Workers.

We would really appreciate if this could be added to the stable version of wrangler, even if undocumented similar to --node-compat for dev. Then it could still be removed or updated later when you figure out the long term strategy around node compatiblity.

Hebilicious · 2023-12-24T11:04:25Z

Hi @janpio!
If I understand correctly, you were you able to deploy a project to cloudflare pages with pg has a dependency, and then you can use postgres from cloudflare pages without issues?

petebacondarwin

This feels like a reasonable addition, given that it is already supported by Pages dev and build commands.

What's left to get this ready to be merged?

janpio · 2024-01-10T00:29:27Z

@Hebilicious Yes, this change to the CLI unblocks usage of pg in Cloudflare Pages functions mostly. (It does not work when you also want to use Next.js, but that is a separate problem: cloudflare/next-on-pages#605)

petebacondarwin · 2024-01-10T14:56:43Z

I think a workaround for now is to use wrangler pages build functions --node-compat build-output-directory=public/_worker.js to generate the Pages Worker and then upload that using wrangler publish public.

Does that work for your use case?

admah · 2024-01-22T16:24:27Z

@WalshyDev it looks like a check failed on a formatting issue. Could you fix that and we should be good to merge?

GregBrimble

I think we still first need to discuss and make a clear decision on how this impacts the source of truth for this currently dash-only config setting. This, although aligns with Workers in some ways by exposing this CLI arg in the deploy command, misaligns with Pages in others, in that this setting can now be configured completely independently of our source-of-truth: the dashboard.

janpio · 2024-02-01T16:30:19Z

@GregBrimble Can node_compat (or --node-compat) actually be configured via the UI/dashboard?

In my (external) understanding node_compat is a build-time flag that influences how the CLI builds the code of your worker (by adding a bunch of polyfills that makes for example pg work), and nodejs_compat is a runtime compatibility flag that influences how the code that has been uploaded is being run on the worker, or what code is available when it runs (like some node APIs in the case of nodejs_compat).

See cloudflare/workers-sdk#2541

* Temproarily disable Worker pages test See cloudflare/workers-sdk#2541 * Fix things * Fix finally * Apply suggestions from code review Co-authored-by: Joël Galeran <Jolg42@users.noreply.github.com> --------- Co-authored-by: Joël Galeran <Jolg42@users.noreply.github.com>

petebacondarwin · 2024-02-19T13:30:53Z

Since this PR is very unlikely to land as-is (e.g. we actually want to make nodejs compat flag the approach to take in the long run) and this PR has been sitting around for quite some time, we are going to close it for now. If and when we decide to implement the node-compat build time setting for Pages we can reopen or reimplement this.

janpio · 2024-02-19T14:11:12Z

What is the recommended workaround in the meantime @petebacondarwin? Right now it seems that pg is not usable with Cloudflare Pages then (as it requires node-compat).

petebacondarwin · 2024-02-19T14:24:14Z

@janpio - can you try the workaround here?

#2541 (comment)

janpio · 2024-02-19T14:41:39Z

Oh sorry, we tried that but seem to have forgotten to report back:

The command there is invalid, I assume the correct order would be wrangler pages functions build --node-compat build-output-directory=public/_worker.js

Then it fails during the publish:

▲ [WARNING] `wrangler publish` is deprecated and will be removed in the next major version.

  Please use `wrangler deploy` instead, which accepts exactly the same arguments.



✘ [ERROR] A compatibility_date is required when publishing. Add the following to your wrangler.toml file:.

      ```
      compatibility_date = "2024-02-01"
      ```
      Or you could pass it in your terminal as `--compatibility-date 2024-02-01`
  See https://developers.cloudflare.com/workers/platform/compatibility-dates for more information.

Full setup here: prisma/ecosystem-tests#4504 + https://github.com/prisma/ecosystem-tests/actions/runs/7743032416/job/21113520833?pr=4504#step:8:147

petebacondarwin · 2024-02-19T14:47:28Z

OK let me reproduce. The error message seems to imply you are only missing --compatibility-date flag?

janpio · 2024-02-19T15:45:43Z

Indeed, seems I got lost in the different commands before.

Although I now realized that wrangler publish publishes a Worker and not Pages. What is required to build the whole Pages "combination" and deploy it? I got lost when trying to use wrangler pages publish with something I previously built with wrangler pages functions build.

petebacondarwin · 2024-02-19T16:16:06Z

Yeah sorry about this. It is all a bit messy right now. Trying to work out if there is a magical combination of commands that we could use.

petebacondarwin · 2024-02-19T16:35:49Z

Give this a go: prisma/ecosystem-tests#4574

millsp · 2024-02-19T21:19:41Z

Coming here to report that Pete's workaround is working well 🚀. Thanks!

Hebilicious · 2024-02-20T13:23:41Z

@petebacondarwin Is there any chance that the workaround could be documented in the Pages docs, or at least here ?
I believe using pg in cloudflare pages is quite common.

This is what I could gather from the Prisma PR :

echo "Temporary disabled, because wrangler does not support "--node-compat" flag for pages commands yet. See https://github.com/cloudflare/workers-sdk/pull/2541"
pnpm install

# First build the functions using the `--node-compat` flag into the `_worker.js` directory
# (Note that the functions are in the non-standard `fns` directory so that they are not confused with the output `_worker.js` directory.)
# Make sure build folder does not exist
rm -rf build
# Build pages function to build/_worker.js
pnpm wrangler pages functions build fns --node-compat --outdir build/_worker.js
# Copy index.html to build folder
cp index.html build/index.html
# Now deploy the _worker.js alongside the index.html asset to Pages
pnpm wrangler pages deploy ./build

delay · 2024-02-26T16:48:27Z

Maybe you should consider updating your documentation because as far as I can tell this still isn't working with pg and Pages. The workaround I have found is to use the postgres npm package instead of pg.

petebacondarwin · 2024-05-02T10:46:23Z

Maybe you should consider updating your documentation because as far as I can tell this still isn't working with pg and Pages. The workaround I have found is to use the postgres npm package instead of pg.

We should update that to make it clear that the tutorial is for Workers not Pages.

WalshyDev requested review from a team as code owners January 12, 2023 23:59

WalshyDev force-pushed the walshy/add-node-compat-to-pages-publish branch from 894c8c0 to bb7f385 Compare January 13, 2023 14:41

JacobMGEvans suggested changes Jan 13, 2023

View reviewed changes

penalosa requested a review from GregBrimble October 2, 2023 15:15

lrapoport-cf assigned nevikashah Oct 2, 2023

GregBrimble previously requested changes Oct 2, 2023

View reviewed changes

lrapoport-cf added the pages Relating to Pages label Oct 2, 2023

WalshyDev and others added 2 commits October 31, 2023 19:11

Add --node-compat to Pages publish

45e0386

Rebase

2bc6376

WalshyDev force-pushed the walshy/add-node-compat-to-pages-publish branch from bb7f385 to 2bc6376 Compare October 31, 2023 19:22

This was referenced Dec 9, 2023

feat(driver-adapters-wasm): Cloudflare Pages projects (cfpages-basic) prisma/ecosystem-tests#4368

Merged

Feature Request: Support --node-compat in wrangler pages build #1074

Closed

petebacondarwin approved these changes Jan 2, 2024

View reviewed changes

GregBrimble requested changes Jan 22, 2024

View reviewed changes

lrapoport-cf added the awaiting Cloudflare response Awaiting response from workers-sdk maintainer team label Feb 7, 2024

SevInf pushed a commit to prisma/ecosystem-tests that referenced this pull request Feb 14, 2024

Temproarily disable Worker pages test

0138e68

See cloudflare/workers-sdk#2541

SevInf mentioned this pull request Feb 14, 2024

Temproarily disable Worker pages test prisma/ecosystem-tests#4556

Merged

petebacondarwin closed this Feb 19, 2024

petebacondarwin mentioned this pull request May 2, 2024

Add note to node-postgres tutorial to highlight it doesn't work on Pages cloudflare/cloudflare-docs#14362

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add --node-compat to Pages publish #2541

Add --node-compat to Pages publish #2541

WalshyDev commented Jan 12, 2023 •

edited

Loading

changeset-bot bot commented Jan 12, 2023 •

edited

Loading

GregBrimble commented Jan 13, 2023

github-actions bot commented Jan 13, 2023 •

edited

Loading

WalshyDev commented Jan 13, 2023

codecov bot commented Jan 13, 2023 •

edited

Loading

JacobMGEvans left a comment

JacobMGEvans Jan 13, 2023

JacobMGEvans commented Jan 13, 2023

notwedtm commented Feb 20, 2023

jadbox commented Sep 29, 2023 •

edited

Loading

GregBrimble left a comment

GregBrimble commented Oct 31, 2023

WalshyDev commented Oct 31, 2023

GregBrimble commented Oct 31, 2023

janpio commented Dec 9, 2023 •

edited

Loading

Hebilicious commented Dec 24, 2023

petebacondarwin left a comment

janpio commented Jan 10, 2024 •

edited

Loading

petebacondarwin commented Jan 10, 2024

admah commented Jan 22, 2024 •

edited

Loading

GregBrimble left a comment

janpio commented Feb 1, 2024

petebacondarwin commented Feb 19, 2024

janpio commented Feb 19, 2024

petebacondarwin commented Feb 19, 2024

janpio commented Feb 19, 2024

petebacondarwin commented Feb 19, 2024

janpio commented Feb 19, 2024 •

edited

Loading

petebacondarwin commented Feb 19, 2024

petebacondarwin commented Feb 19, 2024

millsp commented Feb 19, 2024

Hebilicious commented Feb 20, 2024 •

edited

Loading

delay commented Feb 26, 2024 •

edited

Loading

petebacondarwin commented May 2, 2024

Add --node-compat to Pages publish #2541

Add --node-compat to Pages publish #2541

Conversation

WalshyDev commented Jan 12, 2023 • edited Loading

changeset-bot bot commented Jan 12, 2023 • edited Loading

🦋 Changeset detected

GregBrimble commented Jan 13, 2023

github-actions bot commented Jan 13, 2023 • edited Loading

WalshyDev commented Jan 13, 2023

codecov bot commented Jan 13, 2023 • edited Loading

Codecov Report

JacobMGEvans left a comment

Choose a reason for hiding this comment

JacobMGEvans Jan 13, 2023

Choose a reason for hiding this comment

JacobMGEvans commented Jan 13, 2023

notwedtm commented Feb 20, 2023

jadbox commented Sep 29, 2023 • edited Loading

GregBrimble left a comment

Choose a reason for hiding this comment

GregBrimble commented Oct 31, 2023

WalshyDev commented Oct 31, 2023

GregBrimble commented Oct 31, 2023

janpio commented Dec 9, 2023 • edited Loading

Hebilicious commented Dec 24, 2023

petebacondarwin left a comment

Choose a reason for hiding this comment

janpio commented Jan 10, 2024 • edited Loading

petebacondarwin commented Jan 10, 2024

admah commented Jan 22, 2024 • edited Loading

GregBrimble left a comment

Choose a reason for hiding this comment

janpio commented Feb 1, 2024

petebacondarwin commented Feb 19, 2024

janpio commented Feb 19, 2024

petebacondarwin commented Feb 19, 2024

janpio commented Feb 19, 2024

petebacondarwin commented Feb 19, 2024

janpio commented Feb 19, 2024 • edited Loading

petebacondarwin commented Feb 19, 2024

petebacondarwin commented Feb 19, 2024

millsp commented Feb 19, 2024

Hebilicious commented Feb 20, 2024 • edited Loading

delay commented Feb 26, 2024 • edited Loading

petebacondarwin commented May 2, 2024

WalshyDev commented Jan 12, 2023 •

edited

Loading

changeset-bot bot commented Jan 12, 2023 •

edited

Loading

github-actions bot commented Jan 13, 2023 •

edited

Loading

codecov bot commented Jan 13, 2023 •

edited

Loading

jadbox commented Sep 29, 2023 •

edited

Loading

janpio commented Dec 9, 2023 •

edited

Loading

janpio commented Jan 10, 2024 •

edited

Loading

admah commented Jan 22, 2024 •

edited

Loading

janpio commented Feb 19, 2024 •

edited

Loading

Hebilicious commented Feb 20, 2024 •

edited

Loading

delay commented Feb 26, 2024 •

edited

Loading