CLI initial concept #304

escritorio-gustavo · 2024-04-09T19:35:50Z

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

Find a way to only run ts-rs tests through this CLI
- ❌ Maybe through Ignore ts-rs tests by default #253 and adding the -- --ignored flag to the command
- ✅ Maybe we should gate the generation of tests?
  - ✅ We could add a feature flag to ts-rs that re-enables test generation.
  - ✅ This feature would always be enabled when using this CLI
  - ❌ The entire derive could be turned into a no-op when the feature is off
Explore a way of generating an index.ts file to reexport all the generated types
- ❌ We could, after cargo test finishes, walk the output directory and create an index.ts containing export * 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-metadata or something)?
    - Otherwise, if used without the CLI and with the export feature, 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 test makes the file larger, due to using the append write-mode
    - Users would never manually use this feature, but it would be used by the CLI just like export currently is
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 Combine exports into single file #59 (see feat: Allow multiple types to set #[ts(export_to = "...")] to the same file #316), i.e. allowing multiple export_to attributes 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 from std::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 OpenOptions with both read and write set to true.
        
        Generate the type's imports
        
        In the already existing file, seek for a \n\n sequence 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 export statement
      - If it isn't:
        
        Use fs::write_all like in the current behavior to completely override the file.
        
        Add an entry to the map with the file path and the type name
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 init subcommand to help the user create said config file?
- Whatever else this CLI needs to do that isn't listed here
When we consider it ready, it would be interesting to also make it available through cargo binstall

Feature wishlist

Customize representations (e.g number vs bigint) (@NyxCode) (being worked on in proof-of-concept of type overrides using the CLI #334)
Load configuration from configfile (@NyxCode) (being worked on in proof-of-concept of type overrides using the CLI #334)
(maybe) Combine multiple types in one file? (@NyxCode) (awaiting review in feat: Allow multiple types to set #[ts(export_to = "...")] to the same file #316)
(maybe) generate index.ts file, 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.

NyxCode · 2024-04-10T14:10:42Z

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 test working, or maybe require cargo test -- export_bindings, but to keep it usable without CLI.
Do we want to enable the *-impl features 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 the Cargo.toml.

NyxCode · 2024-04-10T14:15:56Z

Explore a way of generating an index.ts file to reexport all the generated types
- We could, after cargo test finishes, walk the output directory and create an index.ts containing export * 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.

escritorio-gustavo · 2024-04-10T14:26:10Z

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

NyxCode · 2024-04-10T14:53:48Z

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.

NyxCode · 2024-04-10T15:04:05Z

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

NyxCode · 2024-04-10T17:24:46Z

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.)

escritorio-gustavo · 2024-04-10T17:37:23Z

Makes sense! I added an extra check with the cfg!() macro to verify the feature before generating the test

…t value, this way, it'll obey the user's environment variable if it isn't passed into the CLI

escritorio-gustavo · 2024-04-10T19:12:08Z

Customize representations (e.g number vs bigint)

Maybe we should revisit #94 to have a better idea of how to handle this

NyxCode · 2024-04-10T20:22:23Z

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 }

NyxCode · 2024-04-10T20:26:24Z

#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.

NyxCode · 2024-04-11T00:02:24Z

--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" }

escritorio-gustavo · 2024-04-11T12:21:54Z

--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" }

Now that would make it easier lol

NyxCode · 2024-04-11T13:23:36Z

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.
The CLI parses the .ts files to figure out which types are in them
- Lot of duplicated work there
- Alternative: We generate a comment in the first line of the .ts files, and just parse that.
The tests don't generate TS, but json instead, and the CLI does the conversion to TS
The tests generate an additional metadata file, which the CLI will parse

escritorio-gustavo · 2024-04-11T17:15:07Z

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

NyxCode · 2024-04-11T17:44:16Z

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.

escritorio-gustavo · 2024-04-11T19:00:05Z

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

cli/src/main.rs

escritorio-gustavo · 2024-04-12T13:10:15Z

cli/src/main.rs


 use clap::Parser;
+use color_eyre::Result;
+
+mod path;


Had to duplicate ts-rs/src/export/path.rs to make absolute paths

Should we make a ts-rs-core crate to share these types of functions?

cli/src/cargo.rs

CLI initial concept

4570d9e

escritorio-gustavo requested a review from NyxCode April 9, 2024 19:37

Add --add-features flag, fix feature! macro and bson-uuid-impl

7c15180

This comment was marked as resolved.

Sign in to view

Remove *-impl features from CLI

1518c18

This comment was marked as off-topic.

Sign in to view

This comment was marked as resolved.

Sign in to view

escritorio-gustavo added 2 commits April 10, 2024 14:17

Gate test generation behind cargo feature

257d04e

Update Github action

27669f1

escritorio-gustavo mentioned this pull request Apr 10, 2024

Ignore ts-rs tests by default #253

Closed

Make the --output-directory flag an Option instead of having a defaul…

ca3eb23

…t value, this way, it'll obey the user's environment variable if it isn't passed into the CLI

escritorio-gustavo added 2 commits April 12, 2024 10:05

Initial draft on generating index.ts

7914bea

Add missing file

58df89d

escritorio-gustavo commented Apr 12, 2024

View reviewed changes

escritorio-gustavo and others added 7 commits April 15, 2024 08:22

Merge branch 'main' into cli

b402bdc

Clippy warning in test

b1b5cbe

Remove duplicated infer_as test file

6790173

Add error message about naming collisions

50bfc35

Gate metadata file behind feature

88fc140

No longer capture stderr as that would silence serde warnings

66c94dd

Remove changes to infer_as test

1461d3f

escritorio-gustavo commented Apr 17, 2024

View reviewed changes

cli/src/cargo.rs Outdated Show resolved Hide resolved

cli/src/cargo.rs Show resolved Hide resolved

escritorio-gustavo added 2 commits April 17, 2024 10:04

Make --output-directory required

b6c5c65

Make casing consistent

c8a6b14

escritorio-gustavo added CLI This issue/PR is related to or requires the implementation of the CLI tool breaking change This PR will change the public API in some way labels Apr 17, 2024

escritorio-gustavo added 4 commits April 18, 2024 09:28

Merge branch 'main' into cli

fcaa80f

Avoid extra collection allocation

c930c0f

Improve parsing of Metadata

9b19079

Replace new with try_from for clarity

8ff6688

This was referenced Apr 26, 2024

Feature request: How to export multiple types to the same file #314

Closed

Add index.ts with export for all bindings. #133

Open

escritorio-gustavo added 3 commits May 9, 2024 15:25

Add option to export all types to a single file

f229f92

Missing imports

c9b29aa

Fix Syntax Error

89430eb

escritorio-gustavo mentioned this pull request May 10, 2024

Combine exports into single file #59

Closed

NyxCode linked an issue May 17, 2024 that may be closed by this pull request

How to handle naming conflicts #54

Open

escritorio-gustavo and others added 2 commits May 20, 2024 09:12

Merge branch 'main' into cli

147ea87

Merge branch 'main' into cli

00f14af

gustavo-shigueo mentioned this pull request Aug 1, 2024

feat: Allow multiple types to set #[ts(export_to = "...")] to the same file #316

Merged

3 tasks

gustavo-shigueo added 3 commits August 1, 2024 20:20

Merge branch 'main' into cli

7c9643e

Merge branch 'main' into cli

5d72d97

Fix lockfile

5abdda9

karambarakat mentioned this pull request Nov 25, 2024

Feature request: using inventory to build custom bindings #370

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLI initial concept #304

CLI initial concept #304

escritorio-gustavo commented Apr 9, 2024 •

edited by gustavo-shigueo

Loading

NyxCode commented Apr 10, 2024

NyxCode commented Apr 10, 2024

This comment was marked as resolved.

escritorio-gustavo commented Apr 10, 2024 •

edited

Loading

NyxCode commented Apr 10, 2024

This comment was marked as off-topic.

NyxCode commented Apr 10, 2024

This comment was marked as resolved.

NyxCode commented Apr 10, 2024 •

edited

Loading

escritorio-gustavo commented Apr 10, 2024

escritorio-gustavo commented Apr 10, 2024

NyxCode commented Apr 10, 2024

NyxCode commented Apr 10, 2024

NyxCode commented Apr 11, 2024

escritorio-gustavo commented Apr 11, 2024

NyxCode commented Apr 11, 2024

escritorio-gustavo commented Apr 11, 2024

NyxCode commented Apr 11, 2024

escritorio-gustavo commented Apr 11, 2024

escritorio-gustavo Apr 12, 2024

escritorio-gustavo Apr 12, 2024

CLI initial concept #304

Are you sure you want to change the base?

CLI initial concept #304

Conversation

escritorio-gustavo commented Apr 9, 2024 • edited by gustavo-shigueo Loading

Todo

Feature wishlist

Checklist

NyxCode commented Apr 10, 2024

NyxCode commented Apr 10, 2024

This comment was marked as resolved.

escritorio-gustavo commented Apr 10, 2024 • edited Loading

NyxCode commented Apr 10, 2024

This comment was marked as off-topic.

NyxCode commented Apr 10, 2024

This comment was marked as resolved.

NyxCode commented Apr 10, 2024 • edited Loading

escritorio-gustavo commented Apr 10, 2024

escritorio-gustavo commented Apr 10, 2024

NyxCode commented Apr 10, 2024

NyxCode commented Apr 10, 2024

NyxCode commented Apr 11, 2024

escritorio-gustavo commented Apr 11, 2024

NyxCode commented Apr 11, 2024

escritorio-gustavo commented Apr 11, 2024

NyxCode commented Apr 11, 2024

escritorio-gustavo commented Apr 11, 2024

escritorio-gustavo Apr 12, 2024

Choose a reason for hiding this comment

escritorio-gustavo Apr 12, 2024

Choose a reason for hiding this comment

escritorio-gustavo commented Apr 9, 2024 •

edited by gustavo-shigueo

Loading

escritorio-gustavo commented Apr 10, 2024 •

edited

Loading

NyxCode commented Apr 10, 2024 •

edited

Loading