Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(test): enable nodejs compat for miniflare #127

Merged
merged 7 commits into from
Nov 6, 2024
Merged

Conversation

fforbeck
Copy link
Member

@fforbeck fforbeck commented Nov 6, 2024

Fix for Node.js Built-in Module Resolution in Cloudflare Worker Miniflare Tests

Overview

This PR addresses an issue with resolving Node.js built-in modules (node:buffer, node:events, node:async_hooks) in a Cloudflare Worker Test environment. The Worker encountered errors in resolving these built-in modules, which aren’t natively supported in Workers.

This issue was introduced when the Opentelemetry PR was merged, but we didn't see that because the Github Action that runs the test:miniflare was not flagging the test failure.

Error Description

During testing, the following error appeared:

Unable to resolve "dist/worker.mjs" dependency "node:buffer": no matching module rules.
If you're trying to import a Node.js built-in module, or an npm package that uses Node.js built-ins, you'll either need to:
- Bundle your Worker, configuring your bundler to polyfill Node.js built-ins
- Configure your bundler to load Workers-compatible builds by changing the main fields/conditions
- Enable the `nodejs_compat` compatibility flag and use the `NodeJsCompatModule` module type
- Find an alternative package that doesn't require Node.js built-ins

Solution

  1. To resolve this, I've enabled the nodejs_compat compatibility flag in the test/miniflare/freeway.spec.js file - which allows the Cloudflare Miniflare Worker to utilize Node.js runtime APIs, and set the compatibility date to "2024-09-23", as suggested in their docs: https://developers.cloudflare.com/workers/runtime-apis/nodejs/#get-started

  2. Updated the Github Action to halt the execution in case of test failures

  3. Updated the wrangler package to clean up some warnings during the integration tests.

Note

As of September 23, 2024, enabling nodejs_compat applies the same behavior as nodejs_compat_v2 for compatibility dates set to September 23, 2024 or later, meaning this configuration is fully aligned with the latest runtime support for Node.js compatibility in Workers.

@fforbeck fforbeck self-assigned this Nov 6, 2024
@fforbeck fforbeck force-pushed the fix/test-miniflare branch 3 times, most recently from 7b671a8 to 9e1f180 Compare November 6, 2024 17:05
@fforbeck fforbeck removed the request for review from hannahhoward November 6, 2024 17:47
Copy link
Member

@Peeja Peeja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the simpsons flare

.github/actions/test/action.yml Show resolved Hide resolved
@fforbeck fforbeck merged commit 0165521 into main Nov 6, 2024
1 check passed
@fforbeck fforbeck deleted the fix/test-miniflare branch November 6, 2024 18:00
Copy link

@YoutacRandS-VA YoutacRandS-VA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

hannahhoward pushed a commit that referenced this pull request Nov 9, 2024
🤖 I have created a release *beep* *boop*
---


##
[2.21.0](v2.20.2...v2.21.0)
(2024-11-06)


### Features

* **blob-fetcher:** use updated blob fetcher
([#124](#124))
([90bb605](90bb605))
* egress tracker middleware
([#120](#120))
([847829b](847829b))
* rate limiter + unit tests + readme
([#115](#115))
([7bc4c6d](7bc4c6d))


### Bug Fixes

* **test:** enable nodejs compat for miniflare
([#127](#127))
([0165521](0165521))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants