refactor: split languages/clients into packages #92

cameronmoreau · 2021-10-02T21:42:16Z

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:

Create a clean interface for each language/client package to implement
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

cd run-wasm
yarn
yarn bootstrap This will install/build all packages
cd example-nextjs
yarn You may need to npm link each package if you're having trouble
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 · 2021-10-02T21:42:22Z

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 · 2021-10-03T20:21:46Z

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 · 2021-10-03T20:39:41Z

@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 · 2021-10-04T04:58:01Z

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

cameronmoreau · 2021-10-04T13:57:23Z

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 · 2021-10-04T14:26:10Z

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 · 2021-10-04T14:38:38Z

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

jsjoeio · 2021-10-04T16:57:02Z

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 · 2021-10-04T17:01:50Z

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 · 2021-10-05T15:25:35Z

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

@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:

The top level directory is now just a workspace and has base config files each package imports
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 · 2021-10-05T16:01:57Z

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 · 2021-10-05T16:03:12Z

@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 · 2021-10-05T18:26:46Z

@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 · 2021-10-05T18:40:40Z

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 · 2021-10-07T02:57:33Z

@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 · 2021-10-07T17:34:52Z

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 · 2021-10-07T17:50:10Z

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 · 2021-10-07T21:09:02Z

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

allcontributors · 2021-10-07T21:09:10Z

@kennethcassel

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

cameronmoreau added 6 commits October 2, 2021 16:08

add ts package

6c0b712

add python package

44bc587

setup lerna + root package

8d939ca

update example

35ff8d2

re-add lerna dev dep

3842d21

add postinstall for bootstrap

4f813f7

vercel bot had a problem deploying to Preview October 2, 2021 21:42 Failure

cameronmoreau added 4 commits October 2, 2021 20:12

split out wasm core

2429266

fix tsconfig issues

52183e5

fix tests + top level package

b121f16

use packaged run-wasm in example

713094f

vercel bot had a problem deploying to Preview October 3, 2021 15:35 Failure

add bootstrap command

96fe9c3

vercel bot had a problem deploying to Preview October 3, 2021 19:49 Failure

cameronmoreau added 4 commits October 3, 2021 14:53

testing package deploy

47a8441

fix package lock

452ff26

fix lock

1c4f925

use npm prepare intead of postinstall

752499c

vercel bot had a problem deploying to Preview October 3, 2021 20:19 Failure

fix commit linting

4a2a96b

vercel bot had a problem deploying to Preview October 3, 2021 20:29 Failure

vercel bot deployed to Preview October 4, 2021 14:27 View deployment

mcintyre94 mentioned this pull request Oct 5, 2021

Add SQL language evaluation #60

Open

fix gh actions jest

f746b6d

vercel bot deployed to Preview October 5, 2021 15:01 View deployment

run test

e50b1f2

allow dev deps for building

94fb225

vercel bot deployed to Preview October 5, 2021 15:30 View deployment

cameronmoreau changed the title ~~poc: Split languages/clients into packages~~ refactor: split languages/clients into packages Oct 5, 2021

pacakge rename

63687df

vercel bot had a problem deploying to Preview October 5, 2021 16:24 Failure

update lock names

3734578

vercel bot had a problem deploying to Preview October 5, 2021 17:47 Failure

vercel bot deployed to Preview October 7, 2021 17:46 View deployment

kennethcassel merged commit e9b5cf2 into slipHQ:main Oct 7, 2021

allcontributors bot mentioned this pull request Oct 7, 2021

docs: add cameronmoreau as a contributor for code #95

Merged

jsjoeio mentioned this pull request Oct 8, 2021

Packages have both a package-lock.json and a yarn.lock file #96

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: split languages/clients into packages #92

refactor: split languages/clients into packages #92

cameronmoreau commented Oct 2, 2021 •

edited

Loading

vercel bot commented Oct 2, 2021 •

edited

Loading

cameronmoreau commented Oct 3, 2021 •

edited

Loading

cameronmoreau commented Oct 3, 2021

Dhaiwat10 commented Oct 4, 2021

cameronmoreau commented Oct 4, 2021

kennethcassel commented Oct 4, 2021

kennethcassel commented Oct 4, 2021

jsjoeio commented Oct 4, 2021

jsjoeio commented Oct 4, 2021

cameronmoreau commented Oct 5, 2021

kennethcassel commented Oct 5, 2021

kennethcassel commented Oct 5, 2021

jsjoeio commented Oct 5, 2021 •

edited

Loading

kennethcassel commented Oct 5, 2021

cameronmoreau commented Oct 7, 2021

jsjoeio commented Oct 7, 2021

kennethcassel commented Oct 7, 2021

kennethcassel commented Oct 7, 2021

allcontributors bot commented Oct 7, 2021

refactor: split languages/clients into packages #92

refactor: split languages/clients into packages #92

Conversation

cameronmoreau commented Oct 2, 2021 • edited Loading

Intro

Demo

What's next

vercel bot commented Oct 2, 2021 • edited Loading

cameronmoreau commented Oct 3, 2021 • edited Loading

cameronmoreau commented Oct 3, 2021

Dhaiwat10 commented Oct 4, 2021

cameronmoreau commented Oct 4, 2021

kennethcassel commented Oct 4, 2021

kennethcassel commented Oct 4, 2021

jsjoeio commented Oct 4, 2021

jsjoeio commented Oct 4, 2021

cameronmoreau commented Oct 5, 2021

kennethcassel commented Oct 5, 2021

kennethcassel commented Oct 5, 2021

jsjoeio commented Oct 5, 2021 • edited Loading

kennethcassel commented Oct 5, 2021

cameronmoreau commented Oct 7, 2021

jsjoeio commented Oct 7, 2021

kennethcassel commented Oct 7, 2021

kennethcassel commented Oct 7, 2021

allcontributors bot commented Oct 7, 2021

cameronmoreau commented Oct 2, 2021 •

edited

Loading

vercel bot commented Oct 2, 2021 •

edited

Loading

cameronmoreau commented Oct 3, 2021 •

edited

Loading

jsjoeio commented Oct 5, 2021 •

edited

Loading