Repository: Aleph-Alpha/ts-rs
Pull request #256 (2024-03-07 16:19:03)

Alternative handling of output paths
Hey @escritorio-gustavo!

This is an experiment to see if we can further simplify and streamline the handling of export paths.  
Let's start with a practical example:

# Example
## Before
In the workspace E2E test, there's the `parent` crate, which depends on `crate1`.  
The struct `crate1::Crate1` has `#[ts(export_to = "crate1/")]`, and 
the struct `parent::Parent` has `#[ts(export)]`.

When running `cargo test`, only `parent::Parent` exports, but it takes its dependency `crate1::Crate1` with it.
That results in this directory structure:
```
parent/
    bindings/
        Parent.ts
    crate1/
        Crate1.ts
```

## After
With this change, the resulting directory structure looks like this:
```
parent/
    bindings/
        Parent.ts
        crate1/
            Crate1.ts
```

# Explanation
## Before
For a type, the output path is   
`./{TS_RS_EXPORT_DIR}/bindings/{name}.ts` if `#[ts(export_to = "...")]` is **not** specified and
`./{TS_RS_EXPORT_DIR}/{name}.ts` if `#[ts(export_to = "...")]` is specified.

This is because `TS::EXPORT_TO` defaults to `bindings/{name.ts}`, but changing it using `#[ts(export_to = "...")]` gets rid of the `bindings/` prefix.  
The environment variable `TS_RS_EXPORT_DIR` defaults to `.`.

## After
With this change, it's the environment variable `TS_RS_EXPORT_DIR` which defaults to `./bindings`.

Therefore, with this change, the output path is always  `./{TS_RS_EXPORT_DIR}/{name}.ts`.  
We take the `TS_RS_EXPORT_DIR` (or `./bindings`), append the name to it (or the `#[ts(export_to = "...")]` override), and we're done.

---

I feel like this behaviour better matches what users would expect. They specify where there files should go relative to some base directory, `TS_RS_EXPORT_DIR`.  
What do you think?


NyxCode: In our example, `cargo test` does the same as before:  
```
example/
  bindings/
    ...
```
With `TS_RS_EXPORT_DIR=./typescript`, however, you'd get this:
```
example/
  typescript/
    bindings/
      ...
```

With this change, you'd get
```
example/
  typescript/
    ...
```
escritorio-gustavo: > For a type, the output path is
> `./{TS_RS_EXPORT_DIR}/bindings/{name}.ts` if `#[ts(export_to = "...")]` is not specified

Oof, this `bindings` is not supposed to be there, this is a major oversight from my part when doing #250, caused by the fact that all of our tests use `#[ts(export_to = "...")]`


escritorio-gustavo: > ## After
> 
> With this change, it's the environment variable `TS_RS_EXPORT_DIR` which defaults to `./bindings`.
> 
> Therefore, with this change, the output path is always `./{TS_RS_EXPORT_DIR}/{name}.ts`. We take the `TS_RS_EXPORT_DIR` (or `./bindings`), append the name to it (or the `#[ts(export_to = "...")]` override), and we're done.

Very nice way of dealing with it!
NyxCode: Alright, then I'll go ahead and merge this!  
It *is* a breaking change, but it's probably a good time to do it.  
All the tests, for example, have `#[ts(export_to = "tests_out/)]`, so they end up in `ts-rs/bindings/test-out`.

Patch:
diff --git a/example/src/lib.rs b/example/src/lib.rs
index 81e847a..3e5f03b 100644
--- a/example/src/lib.rs
+++ b/example/src/lib.rs
@@ -9,7 +9,7 @@ use uuid::Uuid;
 
 #[derive(Serialize, TS)]
 #[ts(rename_all = "lowercase")]
-#[ts(export, export_to = "bindings/UserRole.ts")]
+#[ts(export, export_to = "UserRole.ts")]
 enum Role {
     User,
     #[ts(rename = "administrator")]
diff --git a/macros/src/lib.rs b/macros/src/lib.rs
index 84704ab..e82f3b5 100644
--- a/macros/src/lib.rs
+++ b/macros/src/lib.rs
@@ -34,17 +34,22 @@ impl DerivedTS {
             .export
             .then(|| self.generate_export_test(&rust_ty, &generics));
 
-        let export_to = {
+        let output_path_fn = {
             let path = match self.export_to.as_deref() {
                 Some(dirname) if dirname.ends_with('/') => {
                     format!("{}{}.ts", dirname, self.ts_name)
                 }
                 Some(filename) => filename.to_owned(),
-                None => format!("bindings/{}.ts", self.ts_name),
+                None => format!("{}.ts", self.ts_name),
             };
 
             quote! {
-                const EXPORT_TO: Option<&'static str> = Some(#path);
+                fn output_path() -> Option<std::path::PathBuf> {
+                    let path = std::env::var("TS_RS_EXPORT_DIR");
+                    let path = path.as_deref().unwrap_or("./bindings");
+
+                    Some(std::path::Path::new(path).join(#path))
+                }
             }
         };
 
@@ -65,7 +70,6 @@ impl DerivedTS {
         quote! {
             #impl_start {
                 #assoc_type
-                #export_to
 
                 fn ident() -> String {
                     #ident.to_owned()
@@ -76,6 +80,7 @@ impl DerivedTS {
                 #decl
                 #inline
                 #generics_fn
+                #output_path_fn
 
                 #[allow(clippy::unused_unit)]
                 fn dependency_types() -> impl ts_rs::typelist::TypeList
diff --git a/ts-rs/src/export.rs b/ts-rs/src/export.rs
index 0dae793..12fe355 100644
--- a/ts-rs/src/export.rs
+++ b/ts-rs/src/export.rs
@@ -1,5 +1,3 @@
-mod path;
-
 use std::{
     any::TypeId,
     collections::BTreeMap,
@@ -8,10 +6,13 @@ use std::{
     sync::Mutex,
 };
 
+pub(crate) use recursive_export::export_type_with_dependencies;
 use thiserror::Error;
 
 use crate::TS;
 
+mod path;
+
 const NOTE: &str = "// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.\n";
 
 /// An error which may occur when exporting a type
@@ -28,7 +29,6 @@ pub enum ExportError {
     ManifestDirNotSet,
 }
 
-pub(crate) use recursive_export::export_type_with_dependencies;
 mod recursive_export {
     use std::{any::TypeId, collections::HashSet};
 
@@ -47,7 +47,7 @@ mod recursive_export {
         fn visit<T: TS + 'static + ?Sized>(&mut self) {
             // if an error occurred previously, or the type cannot be exported (it's a primitive),
             // we return
-            if self.error.is_some() || T::EXPORT_TO.is_none() {
+            if self.error.is_some() || T::output_path().is_none() {
                 return;
             }
 
@@ -86,7 +86,9 @@ mod recursive_export {
 
 /// Export `T` to the file specified by the `#[ts(export_to = ..)]` attribute
 pub(crate) fn export_type<T: TS + ?Sized + 'static>() -> Result<(), ExportError> {
-    let path = output_path::<T>()?;
+    let path = T::output_path()
+        .ok_or_else(std::any::type_name::<T>)
+        .map_err(ExportError::CannotBeExported)?;
     export_type_to::<T, _>(path::absolute(path)?)
 }
 
@@ -133,22 +135,6 @@ pub(crate) fn export_type_to_string<T: TS + ?Sized + 'static>() -> Result<String
     Ok(buffer)
 }
 
-/// Compute the output path to where `T` should be exported.
-pub fn output_path<T: TS + ?Sized>() -> Result<PathBuf, ExportError> {
-    let path = std::env::var("TS_RS_EXPORT_DIR")
-        .ok()
-        .as_deref()
-        .map(Path::new)
-        .unwrap_or_else(|| Path::new("."))
-        .to_owned();
-
-    Ok(path.join(
-        T::EXPORT_TO
-            .ok_or_else(|| std::any::type_name::<T>())
-            .map_err(ExportError::CannotBeExported)?,
-    ))
-}
-
 /// Push the declaration of `T`
 fn generate_decl<T: TS + ?Sized>(out: &mut String) {
     // Type Docs
@@ -164,15 +150,9 @@ fn generate_decl<T: TS + ?Sized>(out: &mut String) {
 
 /// Push an import statement for all dependencies of `T`
 fn generate_imports<T: TS + ?Sized + 'static>(out: &mut String) -> Result<(), ExportError> {
-    let base = std::env::var("TS_RS_EXPORT_DIR")
-        .ok()
-        .as_deref()
-        .map(Path::new)
-        .unwrap_or_else(|| Path::new("."))
-        .to_owned();
-    let export_to =
-        T::EXPORT_TO.ok_or(ExportError::CannotBeExported(std::any::type_name::<T>()))?;
-    let path = base.join(export_to);
+    let path = T::output_path()
+        .ok_or_else(std::any::type_name::<T>)
+        .map_err(ExportError::CannotBeExported)?;
 
     let deps = T::dependencies();
     let deduplicated_deps = deps
diff --git a/ts-rs/src/lib.rs b/ts-rs/src/lib.rs
index 25a4e3f..40a0990 100644
--- a/ts-rs/src/lib.rs
+++ b/ts-rs/src/lib.rs
@@ -83,9 +83,9 @@
 //! | ordered-float-impl | Implement `TS` for types from *ordered_float*                                                                                                                                                             |
 //! | heapless-impl      | Implement `TS` for types from *heapless*                                                                                                                                                                  |
 //! | semver-impl        | Implement `TS` for types from *semver*                                                                                                                                                                    |
-//! 
+//!
 //! <br/>
-//! 
+//!
 //! If there's a type you're dealing with which doesn't implement `TS`, use either
 //! `#[ts(as = "..")]` or `#[ts(type = "..")]`, or open a PR.
 //!
@@ -132,7 +132,6 @@ use std::{
     path::{Path, PathBuf},
 };
 
-pub use export::output_path;
 pub use ts_rs_macros::TS;
 
 pub use crate::export::ExportError;
@@ -154,9 +153,9 @@ pub mod typelist;
 /// Bindings can be exported within a test, which ts-rs generates for you by adding `#[ts(export)]`
 /// to a type you wish to export to a file.
 /// If, for some reason, you need to do this during runtime, you can call [`TS::export`] yourself.
-/// 
+///
 /// **Note:**
-/// Annotating a type with `#[ts(export)]` (or exporting it during runtime using 
+/// Annotating a type with `#[ts(export)]` (or exporting it during runtime using
 /// [`TS::export`]) will cause all of its dependencies to be exported as well.
 ///
 /// ### serde compatibility
@@ -179,12 +178,11 @@ pub mod typelist;
 ///   TS_RS_EXPORT_DIR = { value = "<OVERRIDE_DIR>", relative = true }
 ///   ```
 ///   <br/>
-/// 
+///
 /// - **`#[ts(export_to = "..")]`**  
-///   Specifies where the type should be exported to. Defaults to `bindings/<name>.ts`.  
+///   Specifies where the type should be exported to. Defaults to `<name>.ts`.  
 ///   The path given to the `export_to` attribute is relative to the `TS_RS_EXPORT_DIR` environment variable,
-///   or, if `TS_RS_EXPORT_DIR` is not set, to you project's root directory - more specifically,
-///   it'll be relative to the `Cargo.toml` file.  
+///   or, if `TS_RS_EXPORT_DIR` is not set, to `./bindings`  
 ///   If the provided path ends in a trailing `/`, it is interpreted as a directory.   
 ///   Note that you need to add the `export` attribute as well, in order to generate a test which exports the type.
 ///   <br/><br/>
@@ -299,11 +297,8 @@ pub trait TS {
     /// ```
     type WithoutGenerics: TS + ?Sized;
 
-    /// The path given to `#[ts(export_to = "...")]`
-    const EXPORT_TO: Option<&'static str> = None;
-
     /// JSDoc comment to describe this type in TypeScript - when `TS` is derived, docs are
-    /// automatically read from your doc comments or `#[doc = ".."]` attrubutes
+    /// automatically read from your doc comments or `#[doc = ".."]` attributes
     const DOCS: Option<&'static str> = None;
 
     /// Identifier of this type, excluding generic parameters.
@@ -409,6 +404,20 @@ pub trait TS {
     {
         export::export_type_to_string::<Self>()
     }
+
+    /// Returns the output path to where `T` should be exported.
+    ///
+    /// When deriving `TS`, the output path can be altered using `#[ts(export_to = "...")]`.  
+    /// See the documentation of [`TS`] for more details.
+    /// 
+    /// The output of this function depends on the environment variable `TS_RS_EXPORT_DIR`, which is
+    /// used as base directory. If it is not set, `./bindings` is used as default directory.
+    ///
+    /// If `T` cannot be exported (e.g because it's a primitive type), this function will return
+    /// `None`.
+    fn output_path() -> Option<PathBuf> {
+        None
+    }
 }
 
 /// A typescript type which is depended upon by other types.
@@ -429,11 +438,7 @@ impl Dependency {
     /// If `T` is not exportable (meaning `T::EXPORT_TO` is `None`), this function will return
     /// `None`
     pub fn from_ty<T: TS + 'static + ?Sized>() -> Option<Self> {
-        let exported_to = output_path::<T>()
-            .ok()
-            .as_deref()
-            .and_then(Path::to_str)
-            .map(ToOwned::to_owned)?;
+        let exported_to = T::output_path()?.to_str()?.to_owned();
         Some(Dependency {
             type_id: TypeId::of::<T>(),
             ts_name: T::ident(),


Test patch:
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 133d37b..55d1b77 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -32,13 +32,16 @@ jobs:
         working-directory: e2e/workspace
         run: |
           cargo t 
-          tsc parent/bindings/* --noEmit --noUnusedLocals --strict
+          shopt -s globstar
+          tsc parent/bindings/**/*.ts --noEmit --noUnusedLocals --strict
+          rm -rf parent/bindings
       - name: workspace e2e with default export env
         working-directory: e2e/workspace
         run: |
           TS_RS_EXPORT_DIR=custom-bindings cargo t 
           shopt -s globstar
           tsc parent/custom-bindings/**/*.ts --noEmit --noUnusedLocals --strict
+          rm -rf parent/custom-bindings
   e2e-example:
     name: End-to-end test example
     runs-on: ubuntu-latest
@@ -86,13 +89,16 @@ jobs:
           TS_RS_EXPORT_DIR=output cargo test --no-default-features
           shopt -s globstar
           tsc ts-rs/output/tests-out/**/*.ts --noEmit --noUnusedLocals --strict
+          rm -rf ts-rs/output
       - name: No features
         run: |
           cargo test --no-default-features
           shopt -s globstar
-          tsc ts-rs/tests-out/**/*.ts --noEmit --noUnusedLocals
+          tsc ts-rs/bindings/tests-out/**/*.ts --noEmit --noUnusedLocals
+          rm -rf ts-rs/bindings
       - name: All features
         run: |
           cargo test --all-features
           shopt -s globstar
-          tsc ts-rs/tests-out/**/*.ts --noEmit --noUnusedLocals --strict
+          tsc ts-rs/bindings/tests-out/**/*.ts --noEmit --noUnusedLocals --strict
+          rm -rf ts-rs/bindings
diff --git a/ts-rs/tests/docs.rs b/ts-rs/tests/docs.rs
index 4bd17fb..d6dd34d 100644
--- a/ts-rs/tests/docs.rs
+++ b/ts-rs/tests/docs.rs
@@ -2,7 +2,7 @@
 
 use std::{concat, fs};
 
-use ts_rs::{output_path, TS};
+use ts_rs::TS;
 
 /* ============================================================================================== */
 
@@ -136,7 +136,7 @@ fn export_a() {
         )
     };
 
-    let actual_content = fs::read_to_string(output_path::<A>().unwrap()).unwrap();
+    let actual_content = fs::read_to_string(A::output_path().unwrap()).unwrap();
 
     assert_eq!(actual_content, expected_content);
 }
@@ -182,7 +182,7 @@ fn export_b() {
         )
     };
 
-    let actual_content = fs::read_to_string(output_path::<B>().unwrap()).unwrap();
+    let actual_content = fs::read_to_string(B::output_path().unwrap()).unwrap();
 
     assert_eq!(actual_content, expected_content);
 }
@@ -215,7 +215,7 @@ fn export_c() {
         )
     };
 
-    let actual_content = fs::read_to_string(output_path::<C>().unwrap()).unwrap();
+    let actual_content = fs::read_to_string(C::output_path().unwrap()).unwrap();
 
     assert_eq!(actual_content, expected_content);
 }
@@ -247,7 +247,7 @@ fn export_d() {
             "export type D = null;"
         )
     };
-    let actual_content = fs::read_to_string(output_path::<D>().unwrap()).unwrap();
+    let actual_content = fs::read_to_string(D::output_path().unwrap()).unwrap();
 
     assert_eq!(actual_content, expected_content);
 }
@@ -280,7 +280,7 @@ fn export_e() {
         )
     };
 
-    let actual_content = fs::read_to_string(output_path::<E>().unwrap()).unwrap();
+    let actual_content = fs::read_to_string(E::output_path().unwrap()).unwrap();
 
     assert_eq!(actual_content, expected_content);
 }
@@ -328,7 +328,7 @@ fn export_f() {
         )
     };
 
-    let actual_content = fs::read_to_string(output_path::<F>().unwrap()).unwrap();
+    let actual_content = fs::read_to_string(F::output_path().unwrap()).unwrap();
 
     assert_eq!(actual_content, expected_content);
 }
@@ -376,7 +376,7 @@ fn export_g() {
         )
     };
 
-    let actual_content = fs::read_to_string(output_path::<G>().unwrap()).unwrap();
+    let actual_content = fs::read_to_string(G::output_path().unwrap()).unwrap();
 
     assert_eq!(actual_content, expected_content);
 }
diff --git a/ts-rs/tests/export_manually.rs b/ts-rs/tests/export_manually.rs
index d0491d9..2d985f1 100644
--- a/ts-rs/tests/export_manually.rs
+++ b/ts-rs/tests/export_manually.rs
@@ -2,7 +2,7 @@
 
 use std::{concat, fs};
 
-use ts_rs::{output_path, TS};
+use ts_rs::TS;
 
 #[derive(TS)]
 #[ts(export_to = "tests-out/export_manually/UserFile.ts")]
@@ -36,7 +36,7 @@ fn export_manually() {
         )
     };
 
-    let actual_content = fs::read_to_string(output_path::<User>().unwrap()).unwrap();
+    let actual_content = fs::read_to_string(User::output_path().unwrap()).unwrap();
 
     assert_eq!(actual_content, expected_content);
 }
@@ -57,7 +57,7 @@ fn export_manually_dir() {
         )
     };
 
-    let actual_content = fs::read_to_string(output_path::<UserDir>().unwrap()).unwrap();
+    let actual_content = fs::read_to_string(UserDir::output_path().unwrap()).unwrap();
 
     assert_eq!(actual_content, expected_content);
 }
diff --git a/ts-rs/tests/imports.rs b/ts-rs/tests/imports.rs
index 8f958dd..9a04fbb 100644
--- a/ts-rs/tests/imports.rs
+++ b/ts-rs/tests/imports.rs
@@ -1,5 +1,4 @@
 #![allow(dead_code)]
-use std::path::Path;
 
 use ts_rs::TS;
 
@@ -27,14 +26,7 @@ pub enum TestEnum {
 fn test_def() {
     // The only way to get access to how the imports look is to export the type and load the exported file
     TestEnum::export().unwrap();
-    let path = std::env::var("TS_RS_EXPORT_DIR")
-        .ok()
-        .as_deref()
-        .map(Path::new)
-        .unwrap_or_else(|| Path::new("."))
-        .to_owned()
-        .join(TestEnum::EXPORT_TO.unwrap());
-    let text = std::fs::read_to_string(&path).unwrap();
+    let text = std::fs::read_to_string(TestEnum::output_path().unwrap()).unwrap();
 
     let expected = match (cfg!(feature = "format"), cfg!(feature = "import-esm")) {
         (true, true) => concat!(
@@ -74,5 +66,4 @@ fn test_def() {
     };
 
     assert_eq!(text, expected);
-    std::fs::remove_file(path).unwrap();
 }
diff --git a/ts-rs/tests/path_bug.rs b/ts-rs/tests/path_bug.rs
index 177eb16..c8d6e88 100644
--- a/ts-rs/tests/path_bug.rs
+++ b/ts-rs/tests/path_bug.rs
@@ -1,5 +1,5 @@
 #![allow(dead_code)]
-use ts_rs::{output_path, TS};
+use ts_rs::TS;
 
 #[derive(TS)]
 #[ts(export, export_to = "../ts-rs/tests-out/path_bug/")]
@@ -17,6 +17,6 @@ struct Bar {
 fn path_bug() {
     export_bindings_foo();
 
-    assert!(output_path::<Foo>().unwrap().is_file());
-    assert!(output_path::<Bar>().unwrap().is_file());
+    assert!(Foo::output_path().unwrap().is_file());
+    assert!(Bar::output_path().unwrap().is_file());
 }