workers-sdk icon indicating copy to clipboard operation
workers-sdk copied to clipboard

[WIP] - Promote unstable_dev() to dev()

Open rozenmd opened this issue 2 years ago • 4 comments

This issue will track the issues required to promote api/unstable_dev() to api/dev

  • The API needs to export its types for consumers, tracked by:
    • https://github.com/cloudflare/wrangler2/issues/1387
  • We need to finalise the types that dev uses for the options config:
    • https://github.com/cloudflare/wrangler2/issues/1418
  • We should rewrite our local-mode tests to use unstable_dev:
    • https://github.com/cloudflare/wrangler2/issues/1419
  • //TODO: add more things to this list

rozenmd avatar Jul 06 '22 16:07 rozenmd

I feel like there should be a more defined goal for the librarification of wrangler -- like is the point to export just the commands e.g. wrangler.dev, wrangler.kv.put? or will we also include things like running an authentication flow or parsing a wrangler.toml, which are not "commands" that wrangler runs but are potentially useful to people using wrangler as a library? I guess this is a scope question.

Once that's decided, I think it also makes sense to define how exactly the commands we export are going to be defined -- i think it would be nice to have all the commands follow a certain spec, like for example "every command returns an EventEmitter the user can listen to" or "every command takes as parameters its CLI arguments in camelCase instead ofkebab-case".

I feel fairly strongly that we shouldn't promote dev until these sorts of things are sorted, since any standardization/changes to the public facing...library...API...? would be breaking, and therefore require a new major release?

caass avatar Jul 06 '22 18:07 caass

FWIW @caass we're months away from promoting any of the unstable_ APIs, I just wanted a chunky issue to track the effort involved

rozenmd avatar Jul 07 '22 13:07 rozenmd

FWIW @caass we're months away from promoting any of the unstable_ APIs, I just wanted a chunky issue to track the effort involved

😬 whoops sorry i came off strong, i understand !! i can remove my comment if you like :)

caass avatar Jul 07 '22 13:07 caass

Nah that's cool, we still want to capture ideas around what we want the stable API to look like and whatnot

rozenmd avatar Jul 07 '22 13:07 rozenmd

  • //TODO: add more things to this list

When we import wrangler package, it's sending the wrangler help to the output. To avoid that, I had to do this dirty replacement in the compiled cli.js file:

sed -i 's|  \(main(hideBin(\)|  /**/import_process.default.argv.some(arg => arg.endsWith("wrangler/wrangler-dist/cli.js")) \&\& \1|' $cliPath
 // src/cli.ts
 if (typeof jest === "undefined" && require.main) {
-  main(hideBin(import_process.default.argv)).catch((e2) => {
+  /**/import_process.default.argv.some(arg => arg.endsWith("wrangler/wrangler-dist/cli.js")) && main(hideBin(import_process.default.argv)).catch((e2) => {
     const exitCode = e2 instanceof FatalError && e2.code || 1;
     import_process.default.exit(exitCode);
   });
 }

Is this approach is acceptable, I could send a PR to make this change in the src/cli.ts, if any help is wanted, please let me know.

lmcarreiro avatar Oct 18 '22 02:10 lmcarreiro

Also, it would be good to expose the createTail too. I'm using the same dirty replacement to use it right now to export it:

sed -i 's|  \(unstable_dev: () => unstable_dev\)|  /**/\1,createTail: () => createTail|' $cliPath
 // src/cli.ts
 var cli_exports = {};
 __export(cli_exports, {
-  unstable_dev: () => unstable_dev
+  /**/unstable_dev: () => unstable_dev,createTail: () => createTail
 });
 module.exports = __toCommonJS(cli_exports);
 init_import_meta_url();

If anyone is interested, this is the script I'm using to replace inside cli.js whenever my app starts:

#!/usr/bin/bash

relativeWranglerCliPath='node_modules/wrangler/wrangler-dist/cli.js'
currentDirectory=`pwd`

while
  cliPath="$currentDirectory/$relativeWranglerCliPath"
  currentDirectory=`dirname "$currentDirectory"`
  
  [[ ! -e $cliPath && $currentDirectory != '/' ]]
do true; done

# Add createTail function to the export object in wrangler code
sed -i 's|  \(unstable_dev: () => unstable_dev\)|  /**/\1,createTail: () => createTail|' $cliPath

# Avoid wrangler cli main function if it's running as a library
sed -i 's|  \(main(hideBin(\)|  /**/import_process.default.argv.some(arg => arg.endsWith("wrangler/wrangler-dist/cli.js")) \&\& \1|' $cliPath

lmcarreiro avatar Oct 18 '22 02:10 lmcarreiro

Hey @lmcarreiro,

regarding:

When we import wrangler package, it's sending the wrangler help to the output

That's very weird - could you share more details about your setup?

We're also looking at exposing an unstable_tail API soon - how are you using createTail?

rozenmd avatar Oct 20 '22 13:10 rozenmd

When we import wrangler package, it's sending the wrangler help to the output

That's very weird - could you share more details about your setup?

There is nothing special, if I create a new project I can reproduce that easily:

npm init
echo "const { unstable_dev } = require('wrangler'); console.log(typeof unstable_dev);" > index.js
npm install wrangler --save
node .

image

We're also looking at exposing an unstable_tail API soon - how are you using createTail?

I put the details in the comment above https://github.com/cloudflare/wrangler2/issues/1420#issuecomment-1281725738, it's dirty, but it works. This createTail function is not exported, so I make changes in the node_modules/wrangler/wrangler-dist/cli.js to expose it.

lmcarreiro avatar Oct 20 '22 22:10 lmcarreiro

Hey @lmcarreiro I've resolved the help issue in https://github.com/cloudflare/wrangler2/pull/2052 - if you install [email protected] it should resolve the issue:

Wrote to /Users/rozenmd/Testing/test12/package.json:

{
  "name": "test12",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "keywords": [],
  "author": "Max Rozen",
  "license": "ISC"
}


npm WARN deprecated [email protected]: This package has been deprecated and is no longer maintained. Please use @rollup/plugin-inject.

added 102 packages, and audited 103 packages in 870ms

11 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities
function

rozenmd avatar Oct 21 '22 08:10 rozenmd

Thank you! Now my only dirty change into the cli.js is to expose the createTail. I'll appreciate whenever you expose the unstable_tail API.

lmcarreiro avatar Oct 24 '22 20:10 lmcarreiro

Sorry to necro but as this appears to never have been merged, I assume we are still to use unstable_dev for testing?

JustinGrote avatar Oct 13 '23 20:10 JustinGrote

Yep @JustinGrote unstable_dev is still the way to integration test your Workers. I moved over to the D1 team, so not sure what the progress is here.

rozenmd avatar Oct 13 '23 21:10 rozenmd

We'll be addressing this as part of the Wrangler as a Library work, and so I'm going to close this issue for now. You can track the status of that work here: https://github.com/cloudflare/workers-sdk/milestone/11

penalosa avatar Feb 07 '24 17:02 penalosa