-
Notifications
You must be signed in to change notification settings - Fork 17
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
[ts-sdk][demos] Set up pnpm workspaces, update eslint config, update ts config #830
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good, but not everything builds yet.
btw, web-wasm
package was merged in the mean time so remember to rebase to master
ts/package.json
Outdated
"watch": "lerna run --parallel --stream watch --private", | ||
"lint": "pnpm -r run lint", | ||
"lint:fix": "pnpm -r run lint -- --fix", | ||
"build": "pnpm -r --no-sort --filter ./packages/* run build", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
incorrect glob patter
ts/pnpm-workspace.yaml
Outdated
- 'examples/vite-browser-render' | ||
- 'create-live-compositor' | ||
- 'create-live-compositor/templates/node-minimal' | ||
- 'create-live-compositor/templates/node-express-zustand' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix newlines (few other files also have that)
@@ -1,5 +1,6 @@ | |||
import * as Api from '../api.js'; | |||
import { createCompositorComponent, SceneComponent } from '../component.js'; | |||
import type * as Api from '../api.js'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was type
necessary here for building or is this just a preference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
preference - I like to have it enforced because you're able to easily tell the types and other things apart form each other. Overall, I've placed this rule for every subdir in ts
"@types/react-reconciler": "0.28.8" | ||
}, | ||
"dependencies": { | ||
"react-reconciler": "0.29.2" | ||
}, | ||
"peerDependencies": { | ||
"live-compositor": "^0.1.0" | ||
"react": "^18.3.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"react": "^18.3.1", | |
"react": "*", |
we need to have a large range, if I remember correctly more than one react in app will not work
No description provided.