-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
[tsgen] Expand TS generation test to compile and run a TS file. #22582
base: main
Are you sure you want to change the base?
Conversation
In the new TS file we can add tests for things where just checking if the definition file compiles is not enough. This will help catch issues like emscripten-core#22569.
Bar bar; | ||
std::string string; |
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 changed this to be a more realistic use of a ValueObject.
test/test_other.py
Outdated
|
||
# Test that the output compiles with a TS file that uses the defintions. | ||
cmd = shared.get_npm_cmd('tsc') + ['embind_tsgen.d.ts', '--noEmit'] | ||
shutil.copyfile(test_file('other/embind_tsgen.ts'), 'main.ts') | ||
# A package file with type=module is needed to allow top level await in TS. |
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.
Can you instead use the .mjs
extension?
shutil.copyfile(test_file('other/embind_tsgen.ts'), 'main.ts') | ||
# A package file with type=module is needed to allow top level await in TS. | ||
shutil.copyfile(test_file('other/embind_tsgen_package.json'), 'package.json') | ||
cmd = shared.get_npm_cmd('tsc') + ['embind_tsgen.d.ts', 'main.ts', '--module', 'NodeNext', '--moduleResolution', 'nodenext'] |
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.
Why doesn't tsc generate a file called .mjs
here I wonder?
Is the package.json file here needed for running tsc
or for running the output?
- Use different expected test file now that the output is different. -
In the new TS file we can add tests for things where just checking if the definition file compiles is not enough. This will help catch issues like #22569.