Skip to content

Integrate wally-package-types#235

Open
Waffle3z wants to merge 4 commits intoUpliftGames:mainfrom
Waffle3z:waffle
Open

Integrate wally-package-types#235
Waffle3z wants to merge 4 commits intoUpliftGames:mainfrom
Waffle3z:waffle

Conversation

@Waffle3z
Copy link
Copy Markdown

This change integrates the core behavior of https://github.com/JohnnyMorganz/wally-package-types into wally so that after install, package thunks are automatically mutated to re-export Luau types. No Rojo sourcemap.json is needed; no separate tool or CLI flag is required.

New modules, adapted/copied from wally-package-types: src/require_parser.rs
src/link_mutator.rs (kept for AST helpers/tests; not used by the main pass) src/type_reexports.rs (adapted from wally-package-types command.rs to run with filesystem traversal instead of a sourcemap)

The install flow now performs a best-effort post-pass to mutate thunks and add Luau type re-exports. See Rust.InstallationContext::install(). The pass is invoked automatically and logs warnings rather than failing installation if it encounters an error.

TLS stabilization (Windows):
Switched to rustls-only reqwest in Cargo.toml and wally-registry-backend/Cargo.toml to avoid schannel/native-tls instability. src/package_source/registry.rs constructs a rustls client explicitly.

This change integrates the core behavior of https://github.com/JohnnyMorganz/wally-package-types into wally so that after install, package thunks are automatically mutated to re-export Luau types. No Rojo sourcemap.json is needed; no separate tool or CLI flag is required.

New modules, adapted/copied from wally-package-types:
src/require_parser.rs
src/link_mutator.rs (kept for AST helpers/tests; not used by the main pass)
src/type_reexports.rs (adapted from wally-package-types command.rs to run with filesystem traversal instead of a sourcemap)

The install flow now performs a best-effort post-pass to mutate thunks and add Luau type re-exports. See Rust.InstallationContext::install().
The pass is invoked automatically and logs warnings rather than failing installation if it encounters an error.

TLS stabilization (Windows):
Switched to rustls-only reqwest in Cargo.toml and wally-registry-backend/Cargo.toml to avoid schannel/native-tls instability.
src/package_source/registry.rs constructs a rustls client explicitly.
many minor changes to satisfy `cargo fmt -- --check` and `cargo clippy` to pass the build
Copy link
Copy Markdown
Member

@Dekkonot Dekkonot left a comment

Choose a reason for hiding this comment

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

First off, thank you for opening the PR. I haven't done an in-depth review yet, as I have some overarching feedback first.

  1. Why we don't need a Sourcemap? I don't know why exactly wally-package-types needs one, and I am hesitant to accept a PR that does not use one without knowing why.

  2. This approach feels wrong. It does not feel right to run this after the links are already written using file-system navigation rather than when generating the links. It's potentially more complicated, but it feels backwards when we are the one generating the link files to begin with.

  3. I understand you had to make a bunch of changes to satisfy clippy, but it makes this PR difficult to read. I can work past it by just reviewing the initial commit right now but that won't work as you address feedback. I'd prefer you just revert that change and open a separate PR to fix the clippy issues (or I can do it, I'm not picky).

  4. I'd like some integration tests for this change. It's all well and good to test the individual parts, but I'd like some sort of test that verifies it works as intended when installing packages, especially those with complicated types.

remove post-pass traversal and AST plumbing, drop full_moon, add integration test for typed re-exports
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