refactor: move tablegen to store and worldgen to world, update javascript build/export setup #640

dk1a · 2023-04-18T00:57:31Z

No description provided.

vercel · 2023-04-18T11:18:52Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
mud	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 20, 2023 6:48pm

holic · 2023-04-20T14:34:16Z

packages/common/package.json

+  "type": "module",
+  "exports": {
+    "./codegen": "./dist/codegen/index.js",
+    "./foundry": "./dist/foundry/index.js"


👏 this is nice, so we can import just what we need

I wonder if we should have some policy around deps for this package, to keep it slim/focused

holic · 2023-04-20T14:35:28Z

packages/common/package.json

+  ],
+  "scripts": {
+    "build": "vite build",
+    "release": "npm publish --access=public",


I noticed this sprinkled everywhere - is this needed? (No action needed here, I can clean this up separately if so, just curious)

you mean the release script? It's used by release:manual in the root package.json, which is useful if a lerna release fails midway (after increasing all versions etc) to just push the current state to npm

in that case (assuming all release scripts are the same in each package), we can just recursively exec this same command but from the root package

sounds good! unrelated to this PR though, would leave that for a follow up

holic · 2023-04-20T14:36:33Z

packages/common/src/codegen/render-solidity/userType.ts

@@ -1,25 +1,25 @@
 import { AbiTypeToSchemaType, getStaticByteLength, SchemaType, SchemaTypeToAbiType } from "@latticexyz/schema-type";
 import { StoreConfig, parseStaticArray } from "@latticexyz/config";
-import { RelativeImportDatum, RenderTableType } from "./types.js";
+import { RelativeImportDatum, RenderType } from "./types.js";


I noticed in some areas we're able to remove the .js and others not, is this intentional?

we should be able to remove it everywhere now!

done in 77487c0

holic · 2023-04-20T14:40:33Z

packages/config/package.json

@@ -9,9 +9,10 @@
  },
  "license": "MIT",
  "type": "module",
-  "main": "dist/index.js",


does the removal of main in these packages mean we can import plainly as TS? or does importing end up using the JS build?

in vscode it's gonna resolve types immediately (via the types key), but during build it's gonna resolve to dist/index.js (via the exports key). This means to run tests etc we need to build first (which should be less of a pain once nx is added)

holic · 2023-04-20T14:40:50Z

packages/config/package.json

@@ -21,16 +22,15 @@
    "@latticexyz/schema-type": "^1.42.0",
    "chalk": "^5.2.0",
    "esbuild": "^0.17.15",
-    "ethers": "^6.3.0",
+    "ethers": "^5.7.2",


oof, that would have been bad

holic · 2023-04-20T14:42:56Z

packages/store/package.json

-    "dist": "rimraf abi && mkdir abi && cp -rf out/*.sol/*.json abi/ && pnpm rimraf abi/*.metadata.json",
+    "build": "pnpm run build:js && pnpm run build:tightcoder && pnpm run build:codegen && pnpm run build:abi && pnpm run build:typechain",
+    "build:abi": "forge build --out abi --skip test script",
+    "build:codegen": "node ./dist/tablegen.js",


should we use tsx and the source here?

done in 77487c0

holic · 2023-04-20T14:48:38Z

packages/world/vite.config.ts

+      formats: ["es"],
+    },
+    outDir: "dist",
+    minify: false,


why not minify? there are some perks even on importing in node

no reason, this was just a vite config i copied from another package, can probably enable minify

will leave enabling this everywhere to a followup (in case this breaks something)

holic

looks great! thanks everyone for slogging through this mess to get this over the finish line!

we should probably standardize how we organize packages with both solidity exports and ts/js exports

this PR now adds a ts dir to packages that previously only had solidity in src, which is different from e.g. schema-type that uses src/typescript and src/solidity

edit: added an issue to follow up here: #652

holic · 2023-04-20T14:52:57Z

see https://gist.github.com/sindresorhus/a39789f98801d908bbc7ff3ecc99d99c for CJS vs ESM

dk1a

haha now I'm getting a failed build locally

dk1a · 2023-04-20T13:39:47Z

packages/common/package.json

+  "exports": {
+    "./codegen": "./dist/codegen/index.js",
+    "./foundry": "./dist/foundry/index.js"
+  },


I like this, I was thinking of multiple packages (hence common-codegen), but there's little to no benefit to splitting them I suppose

only benefit I can see is if we need to introduce heavy deps, splitting out one of the "subpackages" into its own proper package may make sense, but we can address that when it comes up

dk1a · 2023-04-20T14:13:55Z

packages/common/src/foundry/index.ts

+/**
+ * Executes the given command, returns the stdout, and logs the command to the console.
+ * Throws an error if the command fails.
+ * @param command The command to execute
+ * @param args The arguments to pass to the command
+ * @returns The stdout of the command
+ */
+async function execLog(command: string, args: string[], options?: Options<string>): Promise<string> {
+  const commandString = `${command} ${args.join(" ")}`;
+  try {
+    console.log(chalk.gray(`running "${commandString}"`));
+    const { stdout } = await execa(command, args, { stdout: "pipe", stderr: "pipe", ...options });
+    return stdout;
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  } catch (error: any) {
+    let errorMessage = error?.stderr || error?.message || "";
+    errorMessage += chalk.red(`\nError running "${commandString}"`);
+    throw new Error(errorMessage);
+  }
+}


will probably want to move this after utils refactoring is done to another common package

dk1a · 2023-04-20T14:29:20Z

packages/std-client/scripts/codegen.mts

@@ -1,8 +1,8 @@
 import { parseStoreConfig } from "@latticexyz/config";
-import { tsgen } from "@latticexyz/cli/dist/render-ts";
+import { tsgen } from "@latticexyz/cli";


btw I was thinking of moving this to recs, just didn't wanna overcomplicate this PR

sounds good!

dk1a · 2023-04-20T14:54:37Z

packages/store/ts/scripts/tablegen.ts

+const config = await loadStoreConfig();
+const srcDir = await getSrcDirectory();
+
+tablegen(config, path.join(srcDir, config.codegenDirectory));


I think this needs await

dk1a · 2023-04-20T15:02:20Z

packages/world/ts/scripts/tablegen.ts

+const config = await loadStoreConfig();
+const srcDir = await getSrcDirectory();
+
+tablegen(config, path.join(srcDir, config.codegenDirectory));


also missing await?

dk1a · 2023-04-20T15:28:48Z

also formatAndWriteSolidity in store/ts/render-solidity/tablegen.ts seems to be missing await, but that's not even related to this PR. I guess I forgot it but things still work cause those functions are quick

This reverts commit 77487c0.

dk1a · 2023-04-20T16:10:30Z

packages/store/ts/modules.d.ts

@@ -0,0 +1 @@
+declare module "prettier-plugin-solidity";


I think this shouldn't be needed in store and world, doing this in just common should be sufficient, as the other packages don't even directly install this plugin (at least locally for me it seems to work)

i had to add this yesterday to solve an error related to it, but can try to see if it also works without it now after shifting other things around 👀

holic · 2023-04-20T16:26:37Z

packages/store/package.json

-    "build": "pnpm codegen && rimraf out && forge build && pnpm dist && pnpm types && tsup",
-    "codegen": "tsx ./scripts/codegen.ts && prettier --write '**/*.sol'",
-    "dist": "rimraf abi && mkdir abi && cp -rf out/*.sol/*.json abi/ && pnpm rimraf abi/*.metadata.json",
+    "build": "pnpm run build:js && pnpm run build:tightcoder && pnpm run build:codegen && pnpm run build:abi && pnpm run build:typechain",


putting build:js first is a bit different than my (ordered) list and expectations here: #633

this might be okay (just the dependencies for this package'sbuild:js step will be different), but wondering if it's okay that we don't run this step after other things (codegen, typechain)

if we have to run it twice, we might want to separate it into two different scripts/names so we can better build the nx pipelines around it

right this wasn't part of my original commit, but as I was wrangling CI I tried building scripts, which had to happen before codegen, and so build:js became first
So unless we move "build:codegen": "node ./dist/tablegen.js", to tsx (I think that had some issues?) it still has to go before codegen

I don't think vite uses codegen's or typechain's results in any way, so it should be fine to just run the whole build:js 1st

a step like this going before is totally fine, we're just going to either need to

be careful about defining dependencies between different steps (build:js happens first here but last in other packages)

separate the "build JS for internal use" and "build JS for external use" steps

dk1a added 2 commits April 18, 2023 03:56

feat: move tablegen to store and worldgen to world

9179ed8

chore: sort-package-json

a633a77

vercel bot had a problem deploying to Preview April 18, 2023 11:18 Failure

fix(network): fix store typechain imports

ea11712

dk1a force-pushed the dk1a/separate-tablegen branch from 819f5fa to ea11712 Compare April 18, 2023 11:26

vercel bot had a problem deploying to Preview April 18, 2023 11:26 Failure

fix(world): add missing package

2f3a768

vercel bot had a problem deploying to Preview April 18, 2023 11:55 Failure

dk1a added 2 commits April 18, 2023 15:42

fix(services): skipLibCheck

9a97d95

fix: fix typechain imports

6baa352

vercel bot had a problem deploying to Preview April 18, 2023 12:56 Failure

fix: attempt tsconfig changes

62f7df7

vercel bot had a problem deploying to Preview April 18, 2023 13:32 Failure

build: try different import

62ffffe

vercel bot had a problem deploying to Preview April 18, 2023 13:52 Failure

vercel bot had a problem deploying to Preview April 18, 2023 15:11 Failure

vercel bot had a problem deploying to Preview April 18, 2023 15:36 Failure

vercel bot had a problem deploying to Preview April 18, 2023 16:02 Failure

dk1a force-pushed the dk1a/separate-tablegen branch from 7d14757 to 1b5d789 Compare April 18, 2023 16:26

vercel bot had a problem deploying to Preview April 18, 2023 16:26 Failure

fix: try externalizing solidity parser

db8544a

dk1a force-pushed the dk1a/separate-tablegen branch from 1b5d789 to db8544a Compare April 18, 2023 18:13

vercel bot had a problem deploying to Preview April 18, 2023 18:13 Failure

Merge branch 'main' into dk1a/separate-tablegen

272ffb6

vercel bot deployed to Preview April 18, 2023 18:32 View deployment

build: try building worldgen script

89eac07

vercel bot deployed to Preview April 18, 2023 18:57 View deployment

build: try to externalize solidity parser more

e503b77

vercel bot deployed to Preview April 18, 2023 19:07 View deployment

holic reviewed Apr 20, 2023

View reviewed changes

holic previously approved these changes Apr 20, 2023

View reviewed changes

holic mentioned this pull request Apr 20, 2023

Standardize our approach for .sol and .ts/.js exports living in the same package #652

Open

dk1a commented Apr 20, 2023

View reviewed changes

refactor: remove .js from ts imports

77487c0

alvrs dismissed holic’s stale review via 77487c0 April 20, 2023 15:28

vercel bot deployed to Preview April 20, 2023 15:29 View deployment

fix: add missing awaits

4f1209a

vercel bot deployed to Preview April 20, 2023 15:36 View deployment

Revert "refactor: remove .js from ts imports"

63d3b58

This reverts commit 77487c0.

vercel bot deployed to Preview April 20, 2023 15:45 View deployment

dk1a commented Apr 20, 2023

View reviewed changes

holic reviewed Apr 20, 2023

View reviewed changes

fix(common): bundle execa

f801ba9

vercel bot deployed to Preview April 20, 2023 18:24 View deployment

build(cli): fix typo in dev script

c64640d

vercel bot deployed to Preview April 20, 2023 18:48 View deployment

dk1a merged commit 60e62eb into main Apr 20, 2023

This was referenced Apr 20, 2023

fix: fix codegen command in release workflow #653

Merged

refactor: normalize vite configs, use tsx for store/world scripts #655

Merged

dk1a deleted the dk1a/separate-tablegen branch April 21, 2023 13:53

alvrs mentioned this pull request Apr 24, 2023

build follow ups #673

Open

18 tasks

refactor: move tablegen to store and worldgen to world, update javascript build/export setup #640

refactor: move tablegen to store and worldgen to world, update javascript build/export setup #640

Conversation

dk1a commented Apr 18, 2023

vercel bot commented Apr 18, 2023 • edited Loading

Choose a reason for hiding this comment

holic Apr 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

holic left a comment • edited Loading

Choose a reason for hiding this comment

holic commented Apr 20, 2023

dk1a left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

holic Apr 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dk1a commented Apr 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

holic Apr 20, 2023 • edited Loading

Choose a reason for hiding this comment

vercel bot commented Apr 18, 2023 •

edited

Loading

holic Apr 20, 2023 •

edited

Loading

holic left a comment •

edited

Loading

holic Apr 20, 2023 •

edited

Loading

dk1a commented Apr 20, 2023 •

edited

Loading

holic Apr 20, 2023 •

edited

Loading