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

Add Cloudflare Workers example using esbuild #217

Closed
saschazar21 opened this issue Nov 28, 2021 · 24 comments
Closed

Add Cloudflare Workers example using esbuild #217

saschazar21 opened this issue Nov 28, 2021 · 24 comments
Labels
enhancement ✨ New feature or request

Comments

@saschazar21
Copy link

Since Cloudflare is deprecating their preconfigured webpack & rust build environments in favor of custom build scripts by setting type = "javascript" in wrangler.toml, I tried bundling the workers code using esbuild, since it's already part of Vite.

I followed the tutorial in the vite-plugin-ssr docs using the provided examples/cloudflare-workers directory in this repo. For the build step, I tried to execute esbuild using the following command:

esbuild worker/index.ts --format=esm --target=es2020 --bundle --outfile=dist/worker/worker.js --platform=browser

Unfortunately, esbuild is complaining that it could not resolve a number of modules, including vite-plugin-ssr in the dist/server/importBuild.js file, as well as of course Node.js-native modules, such as path & stream.

I received similar errors when using webpack@^5.0.0 and the provided webpack.config.js from the cloudflare workers examples.

Is it possible to update and/or add an example for Cloudflare Workers using esbuild as bundler?

SvelteKit's adapter-cloudflare works quite flawlessly, but I haven't found a way to port the functionality to vite-plugin-ssr yet...

@brillout
Copy link
Member

Since Cloudflare is deprecating their preconfigured webpack & rust build environments in favor of custom build scripts by setting type = "javascript" in wrangler.toml,

Is there an official blog post or docs about this? I've seen Clouflare Pages's recent announcement for worker support, but I've not heard of Cloudflare Workers deprecating webpack.

As for esbuild, users have been able to bundle vite-plugin-ssr apps with esbuild before; I believe they used other flag settings than the one you are using. Both Vitedge and Rakkas use esbuild to deploy to Cloudflare Workers, so you may find the right flags there.

Is it possible to update and/or add an example for Cloudflare Workers using esbuild as bundler?

PR welcome :-).

@saschazar21
Copy link
Author

Is there an official blog post or docs about this? I've seen Clouflare Pages's recent announcement for worker support, but I've not heard of Cloudflare Workers deprecating webpack.

Well, technically they're not deprecating it, but rather recommending to specify a custom build section, as noted in the docs for the type property: https://developers.cloudflare.com/workers/cli-wrangler/configuration#keys

Both Vitedge and Rakkas use esbuild to deploy to Cloudflare Workers, so you may find the right flags there.

Thanks for the hint, I will be checking out Vitedge first (haven't found anything concerning Cloudflare Workers in the Rakkas docs at first sight), although the handling under the hood seems to be different than the said importBuild.js approach of vite-plugin-ssr. So the comparison will likely not just end by matching esbuild flags.

@brillout
Copy link
Member

👍 I'm up for esbulid.

So the comparison will likely not just end by matching esbuild flags.

Not sure, could very well be enough.

Keep me updated.

@saschazar21
Copy link
Author

Since I couldn't find any suitable solution for esbuild, I decided to at least try to get it working using webpack@^5.0.0.

The main difference between v4 and v5 is, that the latter doesn't automatically include the necessary Node.js polyfills for the browser anymore (neither does esbuild). Which means they need to be installed manually — so the bundler supports them (esbuild does so via an injected shim file).

Long story short; the webpack configuration below makes the worker files compile, but wrangler isn't able to run them (due to process not being defined, not even when providing the polyfill):

// webpack@^5.0.0 config file

const { resolve } = require('path');

module.exports = {
  mode: 'production',
  entry: './worker/index.js',
  target: 'webworker',
  resolve: {
    fallback: {
	  // needed polyfills, need to be installed manually via npm or yarn.
      buffer: require.resolve('buffer'),
      path: require.resolve('path-browserify'),
      stream: require.resolve('stream-browserify'),
      util: require.resolve('util'),
    },
    mainFields: ['main', 'module'],
    alias: {
	  // needed, because webpack does not understand exports: { '.': { ... } } in the respective package.json files
      'vite-plugin-ssr': require.resolve('vite-plugin-ssr'),
      '@brillout/json-s': require.resolve('@brillout/json-s'),
      '@brillout/libassert': require.resolve('@brillout/libassert'),
    },
  },
  output: {
    path: resolve(__dirname, 'dist/worker'),
    filename: 'worker.js',
  },
};

So far, I see the biggest hurdles in vite-plugin-ssr's dependencies of the Node.js core modules, which all need to be substituted using polyfills for a browser environment like Cloudflare Workers.

@brillout
Copy link
Member

Did you try the --platform=node esbuild flag?

We got pretty far here: https://discord.com/channels/815937377888632913/815937377888632916/911370582447235153

@brillout
Copy link
Member

So far, I see the biggest hurdles in vite-plugin-ssr's dependencies of the Node.js core modules, which all need to be substituted using polyfills for a browser environment like Cloudflare Workers.

Actually, we may remove/replace all Node.js dependencies from vite-plugin-ssr's Node.js runtime. I expect this to be farily easy to achieve.

@saschazar21
Copy link
Author

saschazar21 commented Nov 29, 2021

Yeah, I tried that as well, but since Cloudflare Workers demand a browser environment (since they claim they are technically a Web Worker), I received the following error after executing wrangler dev:

Error: Something went wrong with the request to Cloudflare...
Uncaught ReferenceError: global is not defined
  at line 965 in assertNotAlreadyLoaded
  at line 898 in node_modules/vite-plugin-ssr/dist/cjs/shared/getPageFiles.js
  at line 16 in __require2
  at line 1047 in node_modules/vite-plugin-ssr/dist/cjs/node/page-files/setup.js
  at line 16 in __require2
  at line 4038 in node_modules/vite-plugin-ssr/dist/cjs/node/index.js
  at line 16 in __require2
  at line 11947
 [API code: 10021]

So it's not quite similar to the error mentioned in the Discord channel, but anyhow, I feel like targeting the node platform instead of browser is not quite right here.

What could work though is an intermediate build step - in a way like the following:

  • create a build script which imports the importBuild.js file in a node environment and returns a partially rendered skeleton (including hydration) without any external dependencies (or dependencies pre-bundled), working like an abstract adapter (for AWS Lambda, Next, Cloudflare, whatever ...).
  • have the worker files import the result of that intermediate build step using a dedicated "adapter" (like SvelteKit adapters) for the current environment

I don't want to be the smartypants here, I'm sure that I'm not the only one with an abstract idea like this, but that's what came to my mind in regards to this issue.

@brillout
Copy link
Member

I just checked, we only need a shim for import { isAbsolute, resolve } from 'path' to completely remove Node.js from vite-plugin-ssr's runtime. I transitively control all runtime dependencies. (I wrote the serializer that the plugin uses.)

Provide me with a shim for both and I'll release a version that has no Node.js dependency.

I don't want to be the smartypants here

No probs :-).

@saschazar21
Copy link
Author

path can be substituted with path-browserify, there's even a @types package available, so it fits within your typescript codebase.

Unfortunately stream also needs to be polyfilled, as it is part of vite-plugin-ssr. There's also a polyfill available from browserify, but unfortunately no type definitions. So either the polyfill is cherry-picked out of the browserify/stream-browserify repository and manually fitted with types, or the type definitions are added to the DefinitelyTyped repository. I don't know which option you prefer...

Then there's vite-plugin-ssr itself; esbuild complains about a missing exports declaration in the package.json file (node & import are available, default, browser & require are missing but required by esbuild, when targeting the browser platform). Same applies to brillout/libassert & brillout/json-s. However, this can be fixed by manually adding the require key, as already pointed out in the Discord channel (?).

@brillout
Copy link
Member

Unfortunately stream also needs to be polyfilled, as it is part of vite-plugin-ssr. There's also a polyfill available from browserify, but unfortunately no type definitions. So either the polyfill is cherry-picked out of the browserify/stream-browserify repository and manually fitted with types, or the type definitions are added to the DefinitelyTyped repository. I don't know which option you prefer...

We don't need a Node.js stream polyfill as Cloudflare Workers uses Web Streams. We can dynamicallly load the Node.js stream module only when needed. This circumvents the problem altogether.

path can be substituted with path-browserify, there's even a @types package available, so it fits within your typescript codebase.

We only really need an implementation for resolve.

We can assume all paths to be posix (Vite normalizes all Windows path to Posix).

function isPosixPath(path: string) {
  return !path.includes('\\')
}

function isAbsolute(path: string) {
  assert(isPosixPath(path))
  return path.startsWith('/')
}

function resolve(...paths: string[]) {
  /* TODO */
}

Then there's vite-plugin-ssr itself; esbuild complains about a missing exports declaration in the package.json file (node & import are available, default, browser & require are missing but required by esbuild, when targeting the browser platform). Same applies to brillout/libassert & brillout/json-s. However, this can be fixed by manually adding the require key, as already pointed out in the Discord channel (?).

That shouldn't be a problem.

@saschazar21
Copy link
Author

We don't need a Node.js stream polyfill as Cloudflare Workers uses Web Streams. We can dynamicallly load the Node.js stream module only when needed. This circumvents the problem altogether.

We only really need an implementation for resolve.

I've tried a few things throughout the last few hours, but it seems conditional import and/or tree-shaking doesn't work too well in esbuild.

That means we need a custom implementation for the following path members (since all of them are used across vite-plugin-ssr at some point):

  • join
  • sep
  • dirname
  • isAbsolute
  • relative
  • basename
  • resolve

(Therefore I think it's safer to rely on the browserify's path polyfills altogether, although the resolve polyfill contains a process.cwd(), which causes issues as well... see #217 (comment))

As for the html/streams module, I tried to remove the Node.js-specific imports from the consuming files (like StreamPipeNode, StreamReadableNode, StreamWritableNode, ...) in a first attempt to avoid the Node.js streams module to be included while bundling — but without any luck so far; esbuild keeps on complaining that the streams module could not be resolved.

Other than webpack, esbuild does not seem to provide a fallback mechanism for unresolved modules.

I think I'm stuck here.

@brillout
Copy link
Member

esbuild keeps on complaining that the streams module could not be resolved.

As long as you have import ... from 'stream' and import('stream') statements, esbuild will try to include it in the bundle. (The plan here is to make esbuild ignore stream by doing something like const importName = 'stream'; import(importName);. Esbuild can only statically analyze code.)

That means we need a custom implementation for the following path members (since all of them are used across vite-plugin-ssr at some point):

  • join
  • sep
  • dirname
  • isAbsolute
  • relative
  • basename
  • resolve

These are used by pre-rendering, the runtime only needs resolve and isAbsolute. And actually, I just checked, we don't even need these for Cloudflare Workers.

So seems like we can skip all dependencies to Node.js when bundling for Cloudflare Workers.

I've limited capacity to work on this until I release Telefunc (an RPC implementation I'm working on). So either wait until I realease Telefunc or PR welcome.

In the meantime you can use Cloudflare Worker's webpack bundler right?

@brillout brillout added the enhancement ✨ New feature or request label Nov 29, 2021
@saschazar21
Copy link
Author

saschazar21 commented Nov 30, 2021

I understand there's no external dependency for the importBuild function, but as far as I understood, the renderPage needs to be split into a Node and a Browser part (e.g. when it comes to streams), however I'm not sure how to tackle that (alone), as I have a bit of a hard time understanding all the internal mechanisms of vite-plugin-ssr in detail. So I'm more than happy awaiting your changes, as soon as you've got enough time.

I've limited capacity to work on this until I release Telefunc (an RPC implementation I'm working on). So either wait until I realease Telefunc or PR welcome.

I totally understand and can't wait to try out Telefunc.

In the meantime you can use Cloudflare Worker's webpack bundler right?

Yup, it should fit for now - I just thought there might be a more streamlined approach using esbuild (as it's already built into Vite) or at least webpack@^5.0.0, instead of the overhead of using Cloudflare's own Webpack setup.

@brillout
Copy link
Member

brillout commented Dec 1, 2021

@saschazar21 Thinking of creating a vite-plugin-cloudflare-workers, is that something you'd be interested to work on? (We can co-create it or you can completely create it yourself, whatever you prefer.)

@saschazar21
Copy link
Author

It's definitely an interesting topic, especially when looking at the latest announcements for Wrangler v2 and Cloudflare full-stack - (also already discussed at frandiox/vitedge#68).

However, I'm not sure about creating a new plugin per se — like what would be the purpose? Should it be an enhancement for vite-plugin-ssr, or a standalone ESR environment tailored for Cloudflare, like vitedge?

In terms of Cloudflare Workers, I like the approach of vitedge, but it is a bit too opinionated in terms of the supported frameworks & versions (e.g. vitedge is not yet supporting the latest v6 version of react-router-dom).

So as far as I'm concerned, an 'adapter'-like mechanism could serve vite-plugin-ssr's purpose best, since its only task would be to prepare a file structure for the given deployment environment.

I'd be glad to support, although I have little to no knowledge about Vite's SSR platform, so starting something all on my own without a rough roadmap will probably end in nothing too serious.

@brillout
Copy link
Member

brillout commented Dec 2, 2021

However, I'm not sure about creating a new plugin per se — like what would be the purpose? Should it be an enhancement for vite-plugin-ssr, or a standalone ESR environment tailored for Cloudflare, like vitedge?

Standalone plugin that can be used with any Vite app (with or without vite-plugin-ssr).

FYI vitejs/vite#5936 in case you're up for making a Vite contribution.

Also, I will remove the Node.js dependencies from the runtime tomorrow. We'll then be able to resume our progress on this.

@saschazar21
Copy link
Author

Hi @brillout - thanks for your effort on this topic and sorry for not being that responsive during the past days. I will take a look how different it integrates with my current setup asap. Afterwards I will happily report on the outcome.

I've read your new issue concerning the vite-plugin-cloudflare-workers plugin and I think it's a feasible way to do so. I hope I've got time during the next few days to investigate a bit further in this topic.

@saschazar21
Copy link
Author

I've tried it again using a few more different settings. I think the main issue with esbuild as the bundler for the final Cloudflare Workers build step is how Vite transpiles SSR code.

I'm not entirely sure, but it seems to me, that due to the CommonJS format of the _default.page.server layout, esbuild tries to bundle the whole vite-plugin-ssr package without any tree shaking (same goes for the require statement in importBuild.js).

Since esbuild is configured to bundle for the browser environment, it fails when trying to resolve path and/or stream, independently whether those two packages are needed at runtime or just for the local build.

@brillout
Copy link
Member

brillout commented Dec 7, 2021

Actually simply setting vite.config.js#ssr.noExternal to true could do the trick.

Let me know how it went.

There is also https://tsup.egoist.sh/ but I'm not familiar with it and whether it supports what we need.

@saschazar21
Copy link
Author

I tried with noExternal: true, as well as target: 'webworker', but it still fails due to Node.js built-in dependencies path & stream in vite-plugin-ssr.

I haven't had time to dig deeper into tsup, but after a first look it seems like esbuild enhanced with a configuration file possibility, tailored towards TypeScript projects. Therefore I don't think it has significant advantages over a plain esbuild setup for this particular use case (although could very well be used as drop-in replacement, if custom configs are needed).

@brillout
Copy link
Member

brillout commented Dec 9, 2021

fails due to Node.js built-in dependencies path & stream in vite-plugin-ssr.

I will be removing all Node.js dependencies (/ making them optional) next week. I'll keep you updated.

@brillout
Copy link
Member

FYI cloudflare/wrangler-legacy#2158.

Btw. react-dom-server.node.production.min.js includes require('stream') so we need to shim stream anyways.

There is https://github.com/calvinmetcalf/rollup-plugin-node-builtins.

So

  • Hydrogen is using ssr.noExternal.
  • Vitedge is using esubild
  • Rakkas is using esbuild
  • Not sure about SvelteKit but I believe also esbuild (for sure not webpack)

But none of them seem to be using rollup-plugin-node-builtins. So not sure how they manage to bundle React without a stream shim.

@saschazar21
Copy link
Author

Thanks for the update. I'll be taking a look asap

@brillout
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ✨ New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants