Skip to content
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

Add fixture for declaration transpiler test #708

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MichaelMitchell-at
Copy link
Contributor

I haven't quite figured out how or what to test, but I figure adding a dummy transpiler based on what we have at Airtable could be helpful.


Changes are visible to end-users: no

Test plan

  • Covered by existing test cases

@CLAassistant
Copy link

CLAassistant commented Oct 6, 2024

CLA assistant check
All committers have signed the CLA.

Copy link

aspect-workflows bot commented Oct 6, 2024

Test

1 test target passed

Targets
//ts/test:rootdir_works_with_repeated_directory_typecheck_test [k8-fastbuild] 26ms

Total test execution time was 26ms. 135 tests (99.3%) were fully cached saving 10s.


Buildifier      Format

@MichaelMitchell-at MichaelMitchell-at force-pushed the declaration_transpiler_test_fixture branch 2 times, most recently from 7006a7c to 1e88035 Compare October 6, 2024 23:47
@MichaelMitchell-at MichaelMitchell-at force-pushed the declaration_transpiler_test_fixture branch from 1e88035 to 19b3a9a Compare October 6, 2024 23:48
)

ctx.actions.write(
output = map_file,
content = _DUMMY_SOURCEMAP % src.short_path,
)

return DefaultInfo(files = depset(out_files))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did this have to switch? The transpiler should just have to output the transpiled js or dts (+maps) as default info?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That just means the rule can be used directly for things like js_library(deps) I guess, instead of how ts_project (the macro) or js_library(srcs) uses the DefaultInfo.

I guess it does not harm, but also isn't used in these tests 🤷

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it's probably worth doing in case folks crib this as an example (though we could also provide a production-ready transpiler using oxc as an example ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants