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

request: cloudflare worker support #316

Closed
1 task done
mandar1jn opened this issue Nov 18, 2023 · 5 comments · Fixed by #334
Closed
1 task done

request: cloudflare worker support #316

mandar1jn opened this issue Nov 18, 2023 · 5 comments · Fixed by #334

Comments

@mandar1jn
Copy link

Is there an existing issue or pull request for this?

  • I have searched the existing issues and pull requests

Feature description

From #248 I could gather that cloudflare worker support is something that was being kept in mind. I recently tried to use @discordjs/builders which depends on sapphire and it gave me errors for needing to enable node_compat, something I could gather is heavily discouraged

Desired solution

Implement an alternative for the inspect function imports from util

Alternatives considered

Find another package which provides the same functionality as the inspect function

Additional context

No response

@favna
Copy link
Member

favna commented Nov 18, 2023

I'm not sure where you draw the conclusion from that node_compat is discouraged? The docs don't mention any such thing: https://developers.cloudflare.com/workers/runtime-apis/nodejs

@SuperchupuDev
Copy link

SuperchupuDev commented Dec 7, 2023

@favna the wrangler CLI mentions that node_compat has serious tradeoffs
image

also the plugin it uses is unmaintained

@favna
Copy link
Member

favna commented Dec 8, 2023

Regarding Wrangler using code from ionic, you'll have to take that up with Cloudflare. As for the usage of node:util in this lib, we are very much aware of the issue, but at the same time personally, I have no idea how we can replace it. Possibly @kyranet who designed that part of the code can toss in some ideas. Alternatively, I'm not entirely sure why using @imranbarbhuiya's esbuild-plugins-node-modules-polyfill does not replace var util = require('util'); / import { inspect } from 'util'; from the CJS and ESM builds respectively when the plugin is activated for those bundles. That might be a way to achieve wrangler support as well.

That all said, I'm not sure if just replacing util is enough, because we also depend on:

import get from 'lodash/get.js';
import fastDeepEqual from 'fast-deep-equal/es6/index.js';
import uniqWith from 'lodash/uniqWith.js';

And outside of bundling all the code into the shapeshift bundle I don't see any way to replace those.

@imranbarbhuiya
Copy link
Contributor

wrangler is also planning to use this plugin, maybe it'll solve the issue. see cloudflare/workers-sdk#3832 but they want to ship it as a breaking change so I am not sure if it'll happen anytime soon

@imranbarbhuiya
Copy link
Contributor

Alternatively, I'm not entirely sure why using @imranbarbhuiya's esbuild-plugins-node-modules-polyfill does not replace var util = require('util'); / import { inspect } from 'util'; from the CJS and ESM builds respectively when the plugin is activated for those bundles.

We only enabled it for browser environments as node already have it. We can still make it to bundle for node env too

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 a pull request may close this issue.

4 participants