Exporting of IDL objects from js target. ts-js target added #575
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ined TypeScript file with the JavaScript runtime values
| @@ -126,8 +126,16 @@ pub fn chase_def_use<'a>( | |||
| Ok(res) | |||
| } | |||
|
|
|||
| pub fn chase_types<'a>(env: &'a TypeEnv, tys: &'a [Type]) -> Result<Vec<&'a str>> { | |||
| let mut seen = BTreeSet::new(); | |||
| pub fn chase_types<'a>( | |||
There was a problem hiding this comment.
Let's make it a new function, so that we don't break existing code. It only needs pub(crate) visibility, I think.
| @@ -231,35 +234,55 @@ fn pp_actor<'a>(ty: &'a Type, recs: &'a BTreeSet<&'a str>) -> RcDoc<'a> { | |||
| } | |||
| } | |||
|
|
|||
| pub fn compile(env: &TypeEnv, actor: &Option<Type>) -> String { | |||
| pub fn compile(env: &TypeEnv, actor: &Option<Type>, ts_js: bool) -> String { | |||
There was a problem hiding this comment.
It's probably cleaner to have a new compile function for the ts_js case, even if it means duplicate some of the code.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
didc automatically generates TypeScript and JavaScript files that use
@dfinity/candid. Unfortunately the JavaScript files generated with thejstarget do not export the IDL objects. These objects are useful for example in Azle, as Azle directly uses these objects for all Candid serialization and deserialization. It would be very useful for developers to be able to generate these types from any Candid file. It's useful for Demergent Labs as we can now automatically generate IDL objects for the management canister, ICRC canisters, ICP ledger canister, etc.Another problem with didc currently is that there is no way to generate one TypeScript file that also includes the JavaScript IDL objects. A combined file is very convenient as it would allow developers to import one name from the combined file, and this name could be used as both a TypeScript type and an IDL object. The alternative is to import from two separate files and deal with naming conflicts from the imports.
To accomplish the combined file I have a created a new
ts-jstype. Unfortunately a simple concatenation of the TypeScript and JavaScript target outputs is not sufficient. You will see that the obvious edge cases of these issues (tested across Azle with the management canister, ICP ledger canister, and various ICRC canistesr) have been addressed.Requirements
Developers should be able to run
didcwith--target jsand have all IDL objects exported. They should also be able to rundidcwith--target ts-jsto have one file generated with all TypeScript types and IDL objects exported properly with no TypeScript type errors.Considered Solutions
Exporting the IDL objects by simply duplicating them has been decided as the easiest path forward to get the IDL type objects. Unfortunately there is no code duplication inside of the exported functions from
--target js. The idea is to address this later, as there are complications about which version ofIDLto use since the developer is able to pass their ownIDLin.Simple concatenation for the
--target ts-jswas considered but it unfortunately doesn't work without changes.Recommended Solution
I recommend introducing a new
--targetto produce one combined TypeScript file with the IDL objects and the TypeScript types. I also recommend exporting IDL objects from--target js.Considerations
What impact will this change have on security, performance, users (e.g. breaking changes) or the team (e.g. maintenance costs)?
The complicated changes are from the introduction of
--target ts-js. This allows the changes to--target tsand--target jsto be simpler and hopefully backwards-compatible. The changes to--target jsare relatively simple, as we simply duplicate code and add exports.Tests have not yet been written for the
ts-jstarget. I would like to get some feedback on the PR first.