-
Notifications
You must be signed in to change notification settings - Fork 118
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
[WIP] E2E tests #218
[WIP] E2E tests #218
Conversation
@escritorio-gustavo I think this is the bigger picture we have to keep in mind. Both screnarios which are tested here should just work out-of-the box! |
#[derive(TS)] | ||
pub struct LibraryType { | ||
pub a: i32 | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, since this type doesn't contain #[export]
we'd have to come up with some other way to export it in the consumer crate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right! I'm not sure about how to exactly solve this yet - maybe a #[ts(export)]
type should cause all of its dependencies to be exported as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'd have to make sure it's only exported once, even if the same type is used in a bunch of places
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But this might be confusing, as the user is not explicitly #[ts(export)]
ing the type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But this might be confusing, as the user is not explicitly
#[ts(export)]
ing the type
The problem in that scenario is that the user just cant explicitly export it, since it's in an other crate that may not be under his control.
We'd have to make sure it's only exported once, even if the same type is used in a bunch of places
That's true, this might be the biggest challenge here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem in that scenario is that the user just cant explicitly export it, since it's in an other crate that may not be under his control.
That is true, and it brings up the question I asked in #211:
Should library code in
crates.io
ever implementTS
? Seems like this should be an application code decision (especially the extra dependency + tests)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(let's maybe have this discussion here then)
Should library code in crates.io ever implement TS? Seems like this should be an application code decision (especially the extra dependency + tests)
I think it'd be a good idea to support that, yeah! These libraries would probably never #[ts(export)]
, but making their types implement TS
might be usefull in some cases, I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
e2e/workspace/crate2/src/lib.rs
Outdated
#[derive(TS)] | ||
pub struct Crate2 { | ||
pub x: i32 | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe I've got this to work when #[export, export_to = "..."]
is used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, if all crates of the workspace use #[ts(export_to = "../some_dir")]
, then they all put their files in one directory & the imports work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, and since they are all in the same workspace, it should be easy enough to ensure they are in the same directory.
Although, due to the changes to path diffing, I don't think they even need to be... just #[ts(export)]
might be enough
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just #[ts(export)] might be enough
Haven't tested this yet though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't tested this yet though
Tested it now, it works!
Introduce TypeList, always export Dependencies as well
This PR adds the directory
e2e
, which is supposed to include end-to-end tests to make sure imports / exports are working.I added these two tests to illustrate how the current default behaviour is somewhat broken:
dependencies
A user creates the crate
consumer
, which depends on some cratedependency1
, which possibly lives on crates.io.If a struct in
consumer
contains a type fromdependency1
, it should be exported as well.workspace
A user creates a workspace, containing
crate1
,crate2
, andparent
.crate1
andcrate2
are independent, butparent
depends on bothcrate1
andcrate2
.CI runs
cargo test
andtsc bindings/*
to check if the output is at least valid, which it currently is not for both tests.