CLI initial concept
Hey @NyxCode! This is a very rough first draft for the CLI tool we've been thinking about. It already handles all of our feature flags and the TS_RS_EXPORT_DIR environment variable
Closes #133
Todo
- [x] Find a way to only run
ts-rstests through this CLI- ❌ Maybe through #253 and adding the
-- --ignoredflag to the command - ✅ Maybe we should gate the generation of tests?
- ✅ We could add a feature flag to
ts-rsthat re-enables test generation. - ✅ This feature would always be enabled when using this CLI
- ❌ The entire
derivecould be turned into a no-op when the feature is off
- ✅ We could add a feature flag to
- ❌ Maybe through #253 and adding the
- [x] Explore a way of generating an
index.tsfile to reexport all the generated types- ❌ We could, after
cargo testfinishes, walk the output directory and create an index.ts containingexport * from 'file' - ✅ During export, generate a metadata file to list all types and where they are exported to
- ✅ In the CLI, use the metadata file to create the index.ts file
- ✅ Gate the generation of this file behind a feature (maybe named
generate-metadataor something)?- Otherwise, if used without the CLI and with the
exportfeature, the tests will generate the metadata file and:- This metadata file is never deleted (deletion is only done by the CLI)
- Each run of
cargo testmakes the file larger, due to using theappendwrite-mode
- Users would never manually use this feature, but it would be used by the CLI just like
exportcurrently is
- Otherwise, if used without the CLI and with the
- ❌ We could, after
- [x] Explore a way to merge all exports into a single file
- ✅ Currently being done by reading each file listed in the metadata and copying its data over to
index.ts, then deleting the original - ❓ While adding this feature, I think I thought of a way to solve #59 (see #316), i.e. allowing multiple
export_toattributes with the same file path. I have not written any code for this and have no idea if it will work yet:- Implement a modified version of @NyxCode's experiment mentioned in this comment. Again, I haven't quite thought this through, but I'm thinking a
HashMap<PathBuf, HashSet<String>>. This will map a file path to a set of type names fromstd::any::type_name - For any type with a file export path:
- Check if the path is already in the map
- If it is, check if the type name is in its map entry:
- If it is, skip this type. It has already been exported
- If not:
- Add the type name to the map entry
- Use
OpenOptionswith bothreadandwriteset totrue. - Generate the type's imports
- In the already existing file, seek for a
\n\nsequence and write the imports between the two line feeds (This is important because of how the CLI assumes the file is structured) - Seek to the end of the file and write the type's
exportstatement
- If it isn't:
- Use
fs::write_alllike in the current behavior to completely override the file. - Add an entry to the map with the file path and the type name
- Use
- If it is, check if the type name is in its map entry:
- Check if the path is already in the map
- Implement a modified version of @NyxCode's experiment mentioned in this comment. Again, I haven't quite thought this through, but I'm thinking a
- ✅ Currently being done by reading each file listed in the metadata and copying its data over to
- [ ] Discuss other features of the CLI
- A configuration file so the user doesn't have to pass the flags every time?
- How should the flags interact with the config file?
- Should they be mutually exclusive?
- Should they coexist with flags overriding the file?
- Should there be a config flag to pass the path to the file?
- An
initsubcommand to help the user create said config file?
- How should the flags interact with the config file?
- Whatever else this CLI needs to do that isn't listed here
- A configuration file so the user doesn't have to pass the flags every time?
- [ ] When we consider it ready, it would be interesting to also make it available through
cargo binstall
Feature wishlist
- [ ] Customize representations (e.g
numbervsbigint) (@NyxCode) (being worked on in #334) - [ ] Load configuration from configfile (@NyxCode) (being worked on in #334)
- [x] (maybe) Combine multiple types in one file? (@NyxCode) (awaiting review in #316)
- [x] (maybe) generate
index.tsfile, re-exporting types (@NyxCode) (See explanation above)
Checklist
- [ ] I have followed the steps listed in the Contributing guide.
- [ ] If necessary, I have added documentation related to the changes made.
- [ ] I have added or updated the tests related to the changes made.
Awesome!
For ts-rs, we pretty often do technically breaking releases, but for most users, they are actually drop-in (unless e.g they implement TS manually). For the CLI, we probably want to get that right the first time, so I suggest we carefully plan what we want to support.
I've got a lot of open questions and ideas, but let me start with these two:
- Should the library be still usable without the CLI? I think it'd be a good idea to keep
cargo testworking, or maybe requirecargo test -- export_bindings, but to keep it usable without CLI. - Do we want to enable the
*-implfeatures through the CLI? If we do that, then#[derive(TS)]must be a no-op normally, or code will fail to compile. Even then, if#[derive(TS)]only does something when the CLI is invoked, errors will only show up at that point. As far as I can tell, it'd make more sense for users to enable these features in theCargo.toml.
- [ ] Explore a way of generating an
index.tsfile to reexport all the generated types - We could, aftercargo testfinishes, walk the output directory and create an index.ts containingexport * from 'file'
That'd certainly work. Alternatively, the tests could just output the typescript (or/and metadata) to stdout, and the CLI could maybe combine them, or write the files (and an index.ts) itself.
- Do we want to enable the
*-implfeatures through the CLI? If we do that, then#[derive(TS)]must be a no-op normally, or code will fail to compile. Even then, if#[derive(TS)]only does something when the CLI is invoked, errors will only show up at that point. As far as I can tell, it'd make more sense for users to enable these features in theCargo.toml.
You're right, the *-impl features should not be in the cli
- Should the library be still usable without the CLI? I think it'd be a good idea to keep
cargo testworking, or maybe requirecargo test -- export_bindings, but to keep it usable without CLI.
I think it should, for that we could either merge #253 or add a cargo feature that blocks test generarion, so users would do cargo test --features ts-rs/export or something like that
Alternatively, the tests could just output the typescript (or/and metadata) to stdout, and the CLI could maybe combine them, or write the files (and an
index.ts) itself.
We'd have to find a way to separate what we want from cargo test's normal output (test result: ok. 1 passed; 0 ...)
the tests could just output the typescript
This would need to be behind a new cargo feature otherwise the lib would not be usable without the CLI
add a cargo feature that blocks test generarion, so users would do cargo test --features ts-rs/export or something like that
I like that!
This would need to be behind a new cargo feature otherwise the lib would not be usable without the CLI
Agreed! I'm not sure yet if we need that at all yet, so maybe we should continue to think about features & do that once there's a need for that.
I like that!
Awesome! I'll work on that later today, what do you think the feature should be called?
Awesome! I'll work on that later today, what do you think the feature should be called?
Oh, I'm terrible with names. But export seems reasonable :D
Do you think we should gate only the test generation behind the feature or should we make the whole #[derive(TS)] a no-op?
Do you think we should gate only the test generation behind the feature or should we make the whole #[derive(TS)] a no-op?
I considered both, but I think making #[derive(TS)] a no-op would be pretty intrusive, and potentially break lots of code (custom setups for exporting, custom TS impls with : TS trait bounds, etc.)
Makes sense! I added an extra check with the cfg!() macro to verify the feature before generating the test
- [ ] Customize representations (e.g
numbervsbigint)
Maybe we should revisit #94 to have a better idea of how to handle this
Getting #49359 would be pretty cool - we could parse the JSON output, and do interesting stuff with it.
$ cargo +nightly t --features ts-rs/export -- -Z unstable-options --format json 2>/dev/null
{ "type": "suite", "event": "started", "test_count": 10 }
{ "type": "test", "event": "started", "name": "export_bindings_complexenum" }
{ "type": "test", "event": "started", "name": "export_bindings_complexstruct" }
{ "type": "test", "event": "started", "name": "export_bindings_gender" }
{ "type": "test", "event": "started", "name": "export_bindings_inlinecomplexenum" }
{ "type": "test", "event": "started", "name": "export_bindings_point" }
{ "type": "test", "event": "started", "name": "export_bindings_role" }
{ "type": "test", "event": "started", "name": "export_bindings_series" }
{ "type": "test", "event": "started", "name": "export_bindings_simpleenum" }
{ "type": "test", "event": "started", "name": "export_bindings_user" }
{ "type": "test", "event": "started", "name": "export_bindings_vehicle" }
{ "type": "test", "name": "export_bindings_complexstruct", "event": "ok" }
{ "type": "test", "name": "export_bindings_gender", "event": "ok" }
{ "type": "test", "name": "export_bindings_point", "event": "ok" }
{ "type": "test", "name": "export_bindings_role", "event": "ok" }
{ "type": "test", "name": "export_bindings_simpleenum", "event": "ok" }
{ "type": "test", "name": "export_bindings_vehicle", "event": "ok" }
{ "type": "test", "name": "export_bindings_series", "event": "ok" }
{ "type": "test", "name": "export_bindings_user", "event": "ok" }
{ "type": "test", "name": "export_bindings_complexenum", "event": "ok" }
{ "type": "test", "name": "export_bindings_inlinecomplexenum", "event": "ok" }
{ "type": "suite", "event": "ok", "passed": 10, "failed": 0, "ignored": 0, "measured": 0, "filtered_out": 0, "exec_time": 0.031168 }
{ "type": "suite", "event": "started", "test_count": 0 }
{ "type": "suite", "event": "ok", "passed": 0, "failed": 0, "ignored": 0, "measured": 0, "filtered_out": 0, "exec_time": 0.0001069 }
#49359 seems kinda close. #50297, on the other hand, seems far off. That's unfortunate, since we could probably do really cool stuff with it, without being too hacky.
--show-output together with --format json seems especially cool. Any println! in the tests turns into
{ "type": "test", "name": "some_test", "event": "ok", "stdout": "hey\n" }
--show-outputtogether with--format jsonseems especially cool. Anyprintln!in the tests turns into{ "type": "test", "name": "some_test", "event": "ok", "stdout": "hey\n" }
Now that would make it easier lol
With the "generate an index.ts" feature in mind, the CLI needs some way to get a list of all TS types and their respective .ts files. Solutions I can think of are
- The tests communicate with the CLI by printing to stdout
- That get's messy without
--format json, I'm afraid- Maybe it'd be acceptable to require nightly for the CLI until that's stable, idk.
- That get's messy without
- The CLI parses the
.tsfiles to figure out which types are in them- Lot of duplicated work there
- Alternative: We generate a comment in the first line of the
.tsfiles, and just parse that.
- The tests don't generate
TS, butjsoninstead, and the CLI does the conversion to TS - The tests generate an additional metadata file, which the CLI will parse
- The tests generate an additional metadata file, which the CLI will parse
I think this might be the best way to go. Use the mutex file lock to append to a metadata file all the types' names anc paths, then parse the metadata file to generate the index.ts file
I think this might be the best way to go. Use the mutex file lock to append to a metadata file all the types' names anc paths, then parse the metadata file to generate the index.ts file
I suspect that it might be simpler to write one metadata file per test, but i'd be happy to be convinced otherwise.
I suspect that it might be simpler to write one metadata file per test, but i'd be happy to be convinced otherwise.
I agree that this would probably be simpler, but we'd have to walk the output searching for these metadata files, so we could just walk the diretory reading the ts files instead
This all kinda makes me curious - How would it look like if we tried to do that without a CLI? Gotta play with that during the weekend!
This all kinda makes me curious - How would it look like if we tried to do that without a CLI? Gotta play with that during the weekend!
My guess is that it'd be pretty much impossible, at least the way we've implemented it so far, as it heavily relies on doing stuff after cargo test has finished
My guess is that it'd be pretty much impossible, at least the way we've implemented it so far, as it heavily relies on doing stuff after cargo test has finished
I suspect that's true. For something like example/, I think we could make it work with a static holding the current state, but as soon as there are multiple test executables, like in the ts-rs test suite, stuff get's very complicated.