Skip to content
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

faster dev init, fix wasm clean #985

Merged
merged 6 commits into from
Apr 26, 2024
Merged

faster dev init, fix wasm clean #985

merged 6 commits into from
Apr 26, 2024

Conversation

turbocrime
Copy link
Contributor

@turbocrime turbocrime commented Apr 25, 2024

jesse complained about dev startup slowing down so changing dep from build to compile should accelerate it.

i believe the build issues (package exports) present in 4ef2ef7 that prompted this change are by #995, so there should be no negative effects of this, but watch out for broken HMR or other issues

@turbocrime turbocrime requested a review from jessepinho April 25, 2024 19:06
@turbocrime turbocrime force-pushed the turbocrime/faster-dev branch from 0feed79 to 632074f Compare April 25, 2024 19:16
@turbocrime turbocrime force-pushed the turbocrime/faster-dev branch from 632074f to cc14a83 Compare April 25, 2024 19:18
@turbocrime
Copy link
Contributor Author

threw in a couple script tweaks

@@ -5,7 +5,7 @@ node_modules

# testing
coverage
vite.config.ts.timestamp-*-*.mjs
vite*.config.ts.timestamp-*-*.mjs
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was missing a vitest.config.ts.timestamp...

@@ -8,7 +8,7 @@
"all-check": "pnpm clean && pnpm install && pnpm compile && pnpm lint && pnpm lint:rust && pnpm build && pnpm test && pnpm test:rust",
"build": "turbo build",
"clean": "turbo clean",
"clean:vitest-mjs": "find . -type f -name 'vite.config.ts.timestamp-*-*.mjs' -ls -delete",
"clean:vitest-mjs": "find . -type f -name 'vite*.config.ts.timestamp-*-*.mjs' -ls -delete",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

@@ -4,11 +4,11 @@
"license": "(MIT OR Apache-2.0)",
"type": "module",
"scripts": {
"clean": "rm -v wasm/index*",
"clean": "rm -fv wasm/index*",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prevents failing if this is already clean

"clean:rust": "cargo clean --manifest-path ./crate/Cargo.toml",
"compile": "cd crate && wasm-pack build --no-pack --target bundler --out-name index --out-dir ../wasm",
"dev": "cargo watch -C ./crate --postpone -- pnpm turbo run compile",
"format": "cd crate && cargo fmt --all",
"format:rust": "cd crate && cargo fmt --all",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was already present in turbo.json but not used. now named as intended

Comment on lines 40 to +49
"format": {
"cache": false,
"dependsOn": ["//#format:syncpack", "//#format:prettier", "format:rust"]
"dependsOn": ["format:ts", "format:rust"]
},
"format:rust": {
"dependsOn": ["compile"],
"inputs": ["crate/src/**", "crate/Cargo.toml", "crate/Cargo.lock", "crate/tests/**"]
"cache": false
},
"format:ts": {
"cache": false,
"dependsOn": ["//#format:syncpack", "//#format:prettier"]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

allow option to only format typescript, as format rust has rust dependency. typical use of format command remains the same

Comment on lines 35 to 39
"dev": {
"cache": false,
"dependsOn": ["build"],
"dependsOn": ["^compile"],
"persistent": true
},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dev requires compile of dependencies, and vite will dynamically build all the typescript, so we don't need to depend on build. depending on build only adds a startup delay and demands a perfect build, instead of allowing warnings like dev. so this change allows faster development.

full build requirements are still present in all-check and are enforced by ci, so we lose nothing.

@turbocrime turbocrime changed the title faster dev init faster dev init, fix wasm clean Apr 26, 2024
Copy link
Contributor

@jessepinho jessepinho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EXCELLENT. Thank you! 🙏🏻 😭

@turbocrime
Copy link
Contributor Author

oh hmm i think the build issues are not solved. i am working on it now

@turbocrime turbocrime changed the base branch from main to turbocrime/all-package-exports April 26, 2024 19:15
@turbocrime turbocrime requested a review from jessepinho April 26, 2024 19:16
@turbocrime turbocrime force-pushed the turbocrime/all-package-exports branch from b2ebdda to c082712 Compare April 26, 2024 19:56
@turbocrime
Copy link
Contributor Author

this needs to wait for #995

@turbocrime turbocrime self-assigned this Apr 26, 2024
Base automatically changed from turbocrime/all-package-exports to main April 26, 2024 23:50
@turbocrime turbocrime merged commit 5b21ff1 into main Apr 26, 2024
12 checks passed
@turbocrime turbocrime deleted the turbocrime/faster-dev branch April 26, 2024 23:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants