slipHQ / run-wasm

Run WASM based code executions in the browser easily
https://www.runwasm.com
Apache License 2.0
468 stars 47 forks source link

refactor: split languages/clients into packages #92

Closed cameronmoreau closed 2 years ago

cameronmoreau commented 2 years ago

Saw @zackradisic's comment about splitting up the packages from this project and figured I'd drop a proof-of-concept.

Intro

I think it's a great idea to split up the packages, especially since it looks like each language has a different set of dependencies, and the separation would be easier to maintain.

This splits into two parts:

  1. Create a clean interface for each language/client package to implement
  2. Monorepo infrastructure to split up packages (part 1 of this pr)

A great example of a project that does this is Expo. Check out the packages folder specifically.

Lerna is a great tool to accomplish this, and I've used it in the past for previous large mono-repo package styles projects. For those new to the tool, here's a great example.

Demo

  1. cd run-wasm
  2. yarn
  3. yarn bootstrap This will install/build all packages
  4. cd example-nextjs
  5. yarn You may need to npm link each package if you're having trouble
  6. yarn dev

See the project works with split packages! πŸ™Œ πŸŽ‰

What's next

It looks like each client uses the following, but not sure if loadPackages or a bootstrap should be included.

type RunArgs = {
  code: string
};

interface IWasmClient {
  run(args: RunArgs): Promise<string>;
}

Also curious on folk's opinion for exporting since the example used is

import ts from `@slip/run-run-wasm-ts`

not

import { createTSClient } from '@slip/run-run-wasm-ts'
vercel[bot] commented 2 years ago

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

πŸ” Inspect: https://vercel.com/sliphq/run-wasm/FGfU6SSmqBvvtYJfA3DL6fv5jy28
βœ… Preview: https://run-wasm-git-fork-cameronmoreau-poc-package-breakup-sliphq.vercel.app

[Deployment for 3734578 failed]

cameronmoreau commented 2 years ago

In order to get deploy previews working, you'll have to prepare the other packages, THEN install the demo app.

here's the vercel override install command I used pushd ../ && yarn && yarn bootstrap --no-ci && popd && yarn

Example: https://run-wasm-wheat.vercel.app/

cameronmoreau commented 2 years ago

@kennethcassel and I chatted - Since the "run-wasm" is (for the most part) empty, it may be a great place to add the react interface and any UIs in a follow up Pr.

Ahh okay, then maybe the core run-wasm could be that then? The nice react interface and ui? You just import which clients you need and plug it in, or use the modules by themselves if you’re DIY

React-data-grid structures it that ways. It’s the core, then you install any addons like toolbars and UI

Dhaiwat10 commented 2 years ago

Instead of using this naming convention - run-wasm-python, wouldn't it be a better idea to use something like - @run-wasm/python?

cameronmoreau commented 2 years ago

Instead of using this naming convention - run-wasm-python, wouldn't it be a better idea to use something like - @run-wasm/python?

Totally, I think there's a couple ways. I heard @slip/run-wasm-python as well. I'd love @slipHQ and maintainer friends to chime in on what (slightly permanent) name's yall would like.

kennethcassel commented 2 years ago

Instead of using this naming convention - run-wasm-python, wouldn't it be a better idea to use something like - @run-wasm/python?

Yeah I like this naming convention. Using @run-wasm instead of @slip is preferred. It's not unlikely that we will end up rebranding Slip to another name in the future.

kennethcassel commented 2 years ago

Awesome work @cameronmoreau! Got the vercel build preview working, but looks like the jest tests are failing now

jsjoeio commented 2 years ago

Instead of using this naming convention - run-wasm-python, wouldn't it be a better idea to use something like - @run-wasm/python?

I agree! I like this naming convention too. Plus, it's unlikely you'd be able to get run-wasm-[language] for all packages on npm so having it under an org namespace increases our ability to keep package names consistent.

jsjoeio commented 2 years ago

Hmm...the tests can't run because the tsc check is failing:

$ tsc
src/index.tsx(2,19): error TS2307: Cannot find module 'react' or its corresponding type declarations.
src/index.tsx(20,5): error TS2503: Cannot find namespace 'JSX'.
src/index.tsx(20,5): error TS4060: Return type of exported function has or is using private name 'JSX'.
src/index.tsx(22,5): error TS7026: JSX element implicitly has type 'any' because no interface 'JSX.IntrinsicElements' exists.
src/index.tsx(25,7): error TS7026: JSX element implicitly has type 'any' because no interface 'JSX.IntrinsicElements' exists.
src/index.tsx(25,19): error TS7026: JSX element implicitly has type 'any' because no interface 'JSX.IntrinsicElements' exists.
src/index.tsx(26,5): error TS7026: JSX element implicitly has type 'any' because no interface 'JSX.IntrinsicElements' exists.

Maybe there's an error in the tsconfig? There's an example here

cameronmoreau commented 2 years ago

Love it, what about the main package though? @kennethcassel lmk and I'll update all the package names

  1. @run-wasm/core ?
  2. @run-wasm/run-wasm ??
  3. @run-wasm/react??? Naming is hard

@kennethcassel oops, nice catch. Think I fixed tests. Although I can't figure out how to get them to run in this pr πŸ€” Only Vercel deploy is triggered. yarn test runs fine. Think it was a gh action setup issue.


@jsjoeio are you running tsc to build or check types? Since the packages are split, they each have their own tsconfig/build (so they can be published independently). This means:

  1. The top level directory is now just a workspace and has base config files each package imports
  2. You can't just run tsc or jest top (workspace) level, but rather check-types and test (their alias)

Running yarn check-types and yarn test - Lerna will go into each package and run those commands individually, like the independent packages they are.

Hope this helps. Checkout lerna commands and this example for more info.

kennethcassel commented 2 years ago

Love it, what about the main package though? @kennethcassel lmk and I'll update all the package names

@run-wasm/core ? @run-wasm/run-wasm ?? @run-wasm/react??? Naming is hard

Option 2 is good I think, but yeah naming is hard lol πŸ˜…

kennethcassel commented 2 years ago

@kennethcassel oops, nice catch. Think I fixed tests. Although I can't figure out how to get them to run in this pr πŸ€” Only Vercel deploy is triggered. yarn test runs fine. Think it was a gh action setup issue.

I had to approve and run the tests, actions require maintainer approval for first time contributors

jsjoeio commented 2 years ago

@jsjoeio are you running tsc to build or check types? Since the packages are split, they each have their own tsconfig/build (so they can be published independently). This means:

Thanks for the detailed response! Just check types I guess? We were just using jest to run tests. I believe when it runs, it does type checks as well (i.e. so if you have any TS issues, Jest won't run your tests). Hope that helps. Looks like you got it working just fine πŸ™Œ

kennethcassel commented 2 years ago

Getting this issue on the vercel build

12:48:28.039 Failed to compile.
12:48:28.040 ./pages/index.tsx:2:36
12:48:28.040 Type error: Cannot find module '@run-wasm/python' or its corresponding type declarations.
12:48:28.040 1 | import React, { useEffect, useState } from 'react'
12:48:28.040 > 2 | import { createPythonClient } from '@run-wasm/python'
12:48:28.041 | ^
12:48:28.041 3 | import Editor from '@monaco-editor/react'
12:48:28.041 4 | import Script from 'next/script'
12:48:28.041 5 | import GithubButton from '../components/GithubButton'
12:48:28.063 error Command failed with exit code 1.
12:48:28.063 info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
12:48:28.087 Error: Command "yarn run build" exited with 1
cameronmoreau commented 2 years ago

@kennethcassel did some digging and for some reason once adding the @run-wasm scope, the packages no longer get automatically built when running yarn bootstrap. This is odd because each package has npm run build in their pacakge.json's prepare.

Even more odd because yarn bootstrap properly builds on my machine, but not vercel's deployment. Soooo, as a fix you can add yarn build to the top level repo. I can do some digging on what happened here (because now I'm curious), and publish any findings in a subsequent PR.

TLDR; update the build command (one last time) to -

pushd ../ && pwd && yarn && yarn bootstrap --no-ci && yarn build && popd && yarn
jsjoeio commented 2 years ago

This is odd because each package has npm run build in their pacakge.json's prepare.

@cameronmoreau is this helpful? Wish I had more experience with Lerna to help. Or could it be that you're mixing yarn and npm commands?

kennethcassel commented 2 years ago

TLDR; update the build command (one last time) to -

Trying to test after updating the command but vercel builds are degraded right now! https://www.vercel-status.com/incidents/dtmzmybbbzyh

Will test soon :) hoping to get this one merged in soon! Thanks for all the work @cameronmoreau πŸŽ‰

kennethcassel commented 2 years ago

@all-contributors please add @cameronmoreau for code contributions :)

allcontributors[bot] commented 2 years ago

@kennethcassel

I've put up a pull request to add @cameronmoreau! :tada: