glen/math5
2
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases Wiki Activity

math5 vs dispatch_refactor approach #1

New Issue
Closed
opened 2024-10-03 13:39:54 +00:00 by josdejong · 31 comments
josdejong commented 2024-10-03 13:39:54 +00:00
Collaborator
Copy Link

Thanks for picking this up again @glen! The math5 approach is interesting but like you describe it is really a trade-off with pros and cons.

Some first thoughts on the math5 vs dispatch_refactor approach:

  1. Do I understand it correctly that the builder pattern is a requirement to make this work? Or would it be possible too to change the API of math5 into a simpler factory function like factoryIndependent<CommonSignature<number>>({ add: ... })? The builder API is indeed more more complex which is a pity.

  2. The types generated in the .d.ts files in math5 are beautiful! Really straightforward without any abstraction. I'm not sure though if this really improves on what we have in dispatch_refactor, since we have a (basically) working macro function parseReflectedType there already so all seems fine?

  3. Thinking through a separate build step: I'm not sure how we can relate the types generated in the .d.ts files to the right function implementation via a separate build step. We need to turn these types into something that is runtime available but also map them somehow to the right function. Using a macro solves this by injecting new code at the right place at compile time. Do you see options for such a separate build step?

  4. You're right that there is less duplication in the typing, thanks to handling multiple functions as a group. That is nice indeed. To me, the amount of typing needed in dispatch_refactor is OK already, so this is not a big thing to me, though I understand that you quite like that :).

  5. The thing that I myself dislike most about dispatch_refactor is the need for the $reflect at the bottom. Do you think we can get rid of that by moving it behind the scenes, inside .ship() for example? Still, we could also put a $reflect wrapper around every function which makes things more direct but if I remember well you didn't like that option?

  6. Let's think through how these two approaches will work from an end user point of view: what is needed to understand and write to import a custom function in mathjs. Will it be possible for a user to use this type reflection macro too? What is needed to import a plain JS function?

Thanks for picking this up again @glen! The `math5` approach is interesting but like you describe it is really a trade-off with pros and cons. Some first thoughts on the `math5` vs `dispatch_refactor` approach: 1. Do I understand it correctly that the builder pattern is a requirement to make this work? Or would it be possible too to change the API of `math5` into a simpler factory function like `factoryIndependent<CommonSignature<number>>({ add: ... })`? The builder API is indeed more more complex which is a pity. 2. The types generated in the .d.ts files in `math5` are beautiful! Really straightforward without any abstraction. I'm not sure though if this really improves on what we have in `dispatch_refactor`, since we have a (basically) working macro function `parseReflectedType` there already so all seems fine? 3. Thinking through a separate build step: I'm not sure how we can relate the types generated in the .d.ts files to the right function implementation via a separate build step. We need to turn these types into something that is runtime available but also map them somehow to the right function. Using a macro solves this by injecting new code at the right place at compile time. Do you see options for such a separate build step? 4. You're right that there is less duplication in the typing, thanks to handling multiple functions as a group. That is nice indeed. To me, the amount of typing needed in `dispatch_refactor` is OK already, so this is not a big thing to me, though I understand that you quite like that :). 5. The thing that I myself dislike most about `dispatch_refactor` is the need for the `$reflect` at the bottom. Do you think we can get rid of that by moving it behind the scenes, inside `.ship()` for example? Still, we could also put a `$reflect` wrapper around _every_ function which makes things more direct but if I remember well you didn't like that option? 6. Let's think through how these two approaches will work from an end user point of view: what is needed to understand and write to import a custom function in mathjs. Will it be possible for a user to use this type reflection macro too? What is needed to import a plain JS function?
glen commented 2024-10-03 15:10:02 +00:00
Owner
Copy Link

Thanks for picking this up again @glen! The math5 approach is interesting but like you describe it is really a trade-off with pros and cons.

True, but to me by far the most powerful argument is that math5 actually found typing violations in the implementation code that dispatcher_refactor did not. That suggests that in the dispatcher_refactor version, we are going to lot of trouble to get TypeScript to even run on the implementation definitions, but then we are not getting the value that TypeScript has to offer: stringent type checking that helps uncover errors and potential errors early. In other words, why bother using TypeScript at all if it's not really going to be actively type-checking our code? This is the real reason that I do not feel we should use dispatcher_refactor without changes; either something like math5, or other adjustments (not sure what they would be, but I am always open to tinker) so that TypeScript finds the typing errors (that both we and TypeScript missed in dispatcher_refactor when we were last working on it!).

That said, you raised six specific points, which I will respond to in six comments rather than one über-long one.

> Thanks for picking this up again @glen! The `math5` approach is interesting but like you describe it is really a trade-off with pros and cons. True, but to me _by far_ the most powerful argument is that math5 actually found typing violations in the implementation code that dispatcher_refactor did not. That suggests that in the dispatcher_refactor version, we are going to lot of trouble to get TypeScript to even run on the implementation definitions, but then we are not getting the value that TypeScript has to offer: stringent type checking that helps uncover errors and potential errors early. In other words, why bother using TypeScript at all if it's not really going to be actively type-checking our code? This is the real reason that I do _not_ feel we should use dispatcher_refactor without changes; either something like math5, or other adjustments (not sure what they would be, but I am always open to tinker) so that TypeScript finds the typing errors (that both we and TypeScript missed in dispatcher_refactor when we were last working on it!). That said, you raised six specific points, which I will respond to in six comments rather than one über-long one.
glen commented 2024-10-03 15:24:56 +00:00
Owner
Copy Link
  1. Do I understand it correctly that the builder pattern is a requirement to make this work? Or would it be possible too to change the API of math5 into a simpler factory function like factoryIndependent<CommonSignature>({ add: ... })? The builder API is indeed more more complex which is a pity.

The builder pattern is one way to get narrow typing, that I landed on after a StackOverflow exchange with jcalz, a well known TypeScript typing system guru. What he is pretty sure won't work is something like:

export default implementations<CommonSignature<number>>(
  {dependencies: null, operations: {
      add: blah blah,
      subtract: blah blah,
      etc: ...
  }},
  {dependencies: ['conservativeSqrt', 'complex'], operations: {
      sqrt: blah blah
  })

or in words, just one call to an exporter function with clauses for each of the things you want to export even when they have different dependencies and hence different types to check. Basically jcalz says that the current TypeScript typechecker can't "distribute" typechecking over the properties of an object in the way we would want, to combine each call to .dependent or .independent in the builder into one big call.

I expect that a scheme in which there is one template function call (and perhaps therefore one export) per implementation could also get the typing to be nice and narrow. I think that is basically saying that there would be a way to tweak the dispatcher-refactor less but beef up its typing. I didn't end up there because I was also trying to make the code more dry: not mention the name of each operation at least twice, not reiterate what types we were defining implementations for over and over in each implementation module, and be able to specify the actual factory function as just dep => (a,b) => code which is really simple-looking and clean. But we could look for something closer to dispatcher-refactor that gets the typing really working.

> 1) Do I understand it correctly that the builder pattern is a requirement to make this work? Or would it be possible too to change the API of math5 into a simpler factory function like factoryIndependent<CommonSignature<number>>({ add: ... })? The builder API is indeed more more complex which is a pity. The builder pattern is one way to get narrow typing, that I landed on after a StackOverflow exchange with jcalz, a well known TypeScript typing system guru. What he is pretty sure won't work is something like: ``` export default implementations<CommonSignature<number>>( {dependencies: null, operations: { add: blah blah, subtract: blah blah, etc: ... }}, {dependencies: ['conservativeSqrt', 'complex'], operations: { sqrt: blah blah }) ``` or in words, just one call to an exporter function with clauses for each of the things you want to export even when they have different dependencies and hence different types to check. Basically jcalz says that the current TypeScript typechecker can't "distribute" typechecking over the properties of an object in the way we would want, to combine each call to .dependent or .independent in the builder into one big call. I expect that a scheme in which there is one template function call (and perhaps therefore one export) per implementation could also get the typing to be nice and narrow. I think that is basically saying that there would be a way to tweak the dispatcher-refactor less but beef up its typing. I didn't end up there because I was also trying to make the code more dry: not mention the name of each operation at least twice, not reiterate what types we were defining implementations for over and over in each implementation module, and be able to specify the actual factory function as just `dep => (a,b) => code` which is really simple-looking and clean. But we could look for something closer to dispatcher-refactor that gets the typing really working.
glen commented 2024-10-03 15:34:05 +00:00
Owner
Copy Link
  1. The types generated in the .d.ts files in math5 are beautiful! Really straightforward without any abstraction. I'm not sure though if this really improves on what we have in dispatch_refactor, since we have a (basically) working macro function parseReflectedType there already so all seems fine?

I actually haven't gone back and checked, but my recollection is that parseReflectedType provided somewhat less "grounded" types in that some of our type operations were still embedded in the resulting expressions and we would have to recalculate what they were at runtime. But maybe I am misremembering. In any case, we are both in agreement that in math5, the .d.ts types are pretty much the best we could hope for: They do in places leave some
RealType<T> calls in place, for example, but that seems unavoidable as in those places we don't necessarily know
what type T is, so there is no way to know its RealType. Otherwise, it seems to trace everything back to explicit types, with no extra work on our part. I am not sure that parseReflectedType did quite as well in dispatcher-refactor, and it certainly does much worse in math5.

In any case, the main improvement in math5 is that TypeScript gets to these nice clean types in the .d.ts straight out of the box -- no dependencies, no macros, no remembering "When do I call $reflect!() and when do I call $reflectGen!()?" which frankly is one of the things I found most confusing when trying to look back at dispatcher-refactor and get going with it again.

> 2) The types generated in the .d.ts files in math5 are beautiful! Really straightforward without any abstraction. I'm not sure though if this really improves on what we have in dispatch_refactor, since we have a (basically) working macro function parseReflectedType there already so all seems fine? I actually haven't gone back and checked, but my recollection is that parseReflectedType provided somewhat less "grounded" types in that some of our type operations were still embedded in the resulting expressions and we would have to recalculate what they were at runtime. But maybe I am misremembering. In any case, we are both in agreement that in math5, the .d.ts types are pretty much the best we could hope for: They do in places leave some `RealType<T>` calls in place, for example, but that seems unavoidable as in those places we don't necessarily know what type `T` is, so there is no way to know its RealType. Otherwise, it seems to trace everything back to explicit types, with no extra work on our part. I am not sure that parseReflectedType did quite as well in dispatcher-refactor, and it certainly does much worse in math5. In any case, the _main_ improvement in math5 is that TypeScript gets to these nice clean types in the .d.ts straight out of the box -- no dependencies, no macros, no remembering "When do I call $reflect!() and when do I call $reflectGen!()?" which frankly is one of the things I found most confusing when trying to look back at dispatcher-refactor and get going with it again.
glen commented 2024-10-03 15:41:52 +00:00
Owner
Copy Link
  1. Thinking through a separate build step: I'm not sure how we can relate the types generated in the .d.ts files to the right function implementation via a separate build step. We need to turn these types into something that is runtime available but also map them somehow to the right function. Using a macro solves this by injecting new code at the right place at compile time. Do you see options for such a separate build step?

The macros are just automated code generation that happens to be bundled with a call to a (patched!) tsc. All I would be proposing is a handwritten code-generation step. It could either modify files like arithmetic.js that had been created in the tsc step, or it could make companion files like arithmetic.types.js that we would import when assembling the dispatcher. The latter is slightly more straightforward because it would just be a 1-1 transformation of any blah.d.ts files we find into blah.types.js files, but we could totally do either -- and they will be quite easy, just some light parsing of the .d.ts files, since the types are all just there in pretty much the format we want, we just need to turn the strings into a data structure to be used by the dispatcher-assembling function(s). And that is no more work than in dispatcher-refactor, because we had to write the parseReflectedType mechanism anyway to deal with what the macros were putting out (and looking back at the code for it, that is definitely not trivial -- no reason to expect the second build step will be any worse, codewise).

> 3) Thinking through a separate build step: I'm not sure how we can relate the types generated in the .d.ts files to the right function implementation via a separate build step. We need to turn these types into something that is runtime available but also map them somehow to the right function. Using a macro solves this by injecting new code at the right place at compile time. Do you see options for such a separate build step? The macros are just automated code generation that happens to be bundled with a call to a (patched!) tsc. All I would be proposing is a handwritten code-generation step. It could either modify files like arithmetic.js that had been created in the `tsc` step, or it could make companion files like arithmetic.types.js that we would import when assembling the dispatcher. The latter is slightly more straightforward because it would just be a 1-1 transformation of any blah.d.ts files we find into blah.types.js files, but we could totally do either -- and they will be quite easy, just some light parsing of the .d.ts files, since the types are all just there in pretty much the format we want, we just need to turn the strings into a data structure to be used by the dispatcher-assembling function(s). And that is no more work than in dispatcher-refactor, because we had to write the parseReflectedType mechanism anyway to deal with what the macros were putting out (and looking back at the code for it, that is definitely not trivial -- no reason to expect the second build step will be any worse, codewise).
glen commented 2024-10-03 15:43:42 +00:00
Owner
Copy Link
  1. You're right that there is less duplication in the typing, thanks to handling multiple functions as a group. That is nice indeed. To me, the amount of typing needed in dispatch_refactor is OK already, so this is not a big thing to me, though I understand that you quite like that :).

Ok I didn't really need to respond to this one, we are in mutual understanding on this. Just putting this here so it doesn't seem like I missed one ;-)

> 4) You're right that there is less duplication in the typing, thanks to handling multiple functions as a group. That is nice indeed. To me, the amount of typing needed in dispatch_refactor is OK already, so this is not a big thing to me, though I understand that you quite like that :). Ok I didn't really need to respond to this one, we are in mutual understanding on this. Just putting this here so it doesn't seem like I missed one ;-)
glen commented 2024-10-03 15:59:48 +00:00
Owner
Copy Link
  1. The thing that I myself dislike most about dispatch_refactor is the need for the $reflect at the bottom. Do you think we can get rid of that by moving it behind the scenes, inside .ship() for example? Still, we could also put a $reflect wrapper around every function which makes things more direct but if I remember well you didn't like that option?

I am a little confused by this question, so let me just say:

(a) There is no need for the $reflect!() calls in math5 at all; I just added them to see what types they would produce now that TypeScript was itself producing essentially perfect types, in case that would automatically take care of the "second build step" without our having to explicitly write code for it. But in fact, in math5 $reflect!() produced much less good types; and writing that second build step would be no more work than the parsing we would have to do of $reflect!() output anyway. So in the end I consider $reflect!() in math5 a failed experiment, and would just take it out in favor of writing a simple .d.ts extractor. I only left it there in the repository at the moment so that you could see its operation and results.

(b) In dispatcher-refactor, it appears we need the $reflect!() calls somewhere because the types TypeScript is assigning in .d.ts do not seem adequate (which as I pointed out in my first response, now seems to me to be a very significant shortcoming of dispatcher-refactor as it currently stands, basically implying we are wasting our time using TypeScript there). As a stylistic matter, I somewhat prefer a single $reflect!() call at the end to avoid verboseness, and you prefer multiple $reflect!() calls to avoid repeating the list of implementations provided -- but I do see your point there, that is definitely non-DRY. So the macro call is a pain either way; if we end up with a solution that uses macro calls, I could go either way with where to place them.

My overriding desire is to try to get as close to "just state the dependencies and the code for each implementation factory" as possible. Ideally we would just minimize all the other noise/repetition. I think it is what we strove for in dispatcher-refactor, I just now feel we circumvented TypeScript too much to make it worthwhile, even though at the same time we had to repeat types over and over again! The builder pattern appears to fix those things, at the admitted cost of introducing its own sort of boilerplate verbosity unfortunately.

> 5) The thing that I myself dislike most about dispatch_refactor is the need for the $reflect at the bottom. Do you think we can get rid of that by moving it behind the scenes, inside .ship() for example? Still, we could also put a $reflect wrapper around every function which makes things more direct but if I remember well you didn't like that option? I am a little confused by this question, so let me just say: (a) There is no need for the $reflect!() calls in math5 at all; I just added them to see what types they would produce now that TypeScript was itself producing essentially perfect types, in case that would automatically take care of the "second build step" without our having to explicitly write code for it. But in fact, in math5 $reflect!() produced much less good types; and writing that second build step would be no more work than the parsing we would have to do of $reflect!() output anyway. So in the end I consider $reflect!() in math5 a failed experiment, and would just take it out in favor of writing a simple .d.ts extractor. I only left it there in the repository at the moment so that you could see its operation and results. (b) In dispatcher-refactor, it appears we _need_ the $reflect!() calls somewhere because the types TypeScript is assigning in .d.ts do not seem adequate (which as I pointed out in my first response, now seems to me to be a very significant shortcoming of dispatcher-refactor as it currently stands, basically implying we are wasting our time using TypeScript there). As a stylistic matter, I somewhat prefer a single $reflect!() call at the end to avoid verboseness, and you prefer multiple $reflect!() calls to avoid repeating the list of implementations provided -- but I do see your point there, that is definitely non-DRY. So the macro call is a pain either way; if we end up with a solution that uses macro calls, I could go either way with where to place them. My overriding desire is to try to get as close to "just state the dependencies and the code for each implementation factory" as possible. Ideally we would just minimize all the other noise/repetition. I think it is what we strove for in dispatcher-refactor, I just now feel we circumvented TypeScript too much to make it worthwhile, even though at the same time we had to repeat types over and over again! The builder pattern appears to fix those things, at the admitted cost of introducing its own sort of boilerplate verbosity unfortunately.
glen commented 2024-10-03 16:09:34 +00:00
Owner
Copy Link
  1. Let's think through how these two approaches will work from an end user point of view: what is needed to understand and write to import a custom function in mathjs. Will it be possible for a user to use this type reflection macro too? What is needed to import a plain JS function?

Well, as I said, if we go with a second build step, then we don't need macros in math5. If someone is cloning a math5-based mathjs and modifying it, the second build step will be in place. If someone is using mathjs as a package and wants to extend it by adding another module of TypeScript functions like in the core, we will have to make sure the package also exports the builder and they will have to call it in their build process. And finally, to add some more operations written in plain JavaScript, they will have to call one of the dispatcher-assembly functions with their factory function and the (likely hand-written) data structure describing its dependencies and signature -- that last bit seems exactly the same as in the the dispatcher-refactor case.

Correspondingly, in the dispatcher-refactor case, cloning&modifying mathjs should be fine, the new code just needs to follow the pattern of $reflect! calls of the old. Using mathjs as a package and extending it by new TypeScript modules would entail adding ts-macros to their build dependencies, and importing our $reflect!() (and $reflectGen!() if needed) macros, which would presumably be exported somewhere from the package, and calling them appropriately.

So I don't really see that there's too much difference between dispatcher-refactor and math5 on this particular score...

> 6) Let's think through how these two approaches will work from an end user point of view: what is needed to understand and write to import a custom function in mathjs. Will it be possible for a user to use this type reflection macro too? What is needed to import a plain JS function? Well, as I said, if we go with a second build step, then we don't need macros in math5. If someone is cloning a math5-based mathjs and modifying it, the second build step will be in place. If someone is using mathjs as a package and wants to extend it by adding another module of TypeScript functions like in the core, we will have to make sure the package also exports the builder and they will have to call it in their build process. And finally, to add some more operations written in plain JavaScript, they will have to call one of the dispatcher-assembly functions with their factory function and the (likely hand-written) data structure describing its dependencies and signature -- that last bit seems exactly the same as in the the dispatcher-refactor case. Correspondingly, in the dispatcher-refactor case, cloning&modifying mathjs should be fine, the new code just needs to follow the pattern of $reflect! calls of the old. Using mathjs as a package and extending it by new TypeScript modules would entail adding ts-macros to their build dependencies, and importing our $reflect!() (and $reflectGen!() if needed) macros, which would presumably be exported somewhere from the package, and calling them appropriately. So I don't really see that there's too much difference between dispatcher-refactor and math5 on this particular score...
glen commented 2024-10-03 16:13:12 +00:00
Owner
Copy Link
  1. There was no number 7 but I am adding one ;-)

You didn't comment on the change from (for example) divideReal defined as an "alias" of divide but then treated like its own operation in dispatcher-refactor, vs. in math5, only having divide and just explicitly specifying which signature it is being defined for in each case. Personally looking back, I found the former tricky to follow/read, and the latter clearer. I would suggest that even if we end up in a place closer to dispatcher-refactor, that we revisit (and try to eliminate) this "alias" system. I think it makes it more difficult for someone new to get into the code, so to speak.

> 7) There was no number 7 but I am adding one ;-) You didn't comment on the change from (for example) `divideReal` defined as an "alias" of divide but then treated like its own operation in dispatcher-refactor, vs. in math5, only having `divide` and just explicitly specifying which signature it is being defined for in each case. Personally looking back, I found the former tricky to follow/read, and the latter clearer. I would suggest that even if we end up in a place closer to dispatcher-refactor, that we revisit (and try to eliminate) this "alias" system. I think it makes it more difficult for someone new to get into the code, so to speak.
josdejong commented 2024-10-07 16:01:37 +00:00
Author
Collaborator
Copy Link

to me by far the most powerful argument is that math5 actually found typing violations in the implementation code that dispatcher_refactor did not.

Agree, you're right, that is a strong argument in favor of math5 for me too.

(1, 7) Clear, thanks. In short: the builder pattern is a neccesity. That can be ok with me. How complex an API is I think is mostly how many concepts the user needs to userstand, needing to call a .ship() or .build() is not perse more complex (and a chained API is easier discoverable if you use an IDE with intellisense, so it explains itself). Not needing special constructs like "alias" really makes the API simpler, thats indeed great.

(2, 3, 5, 6) Thanks, I now better understand how math5 works and that it does not neccesary need a TypeScript plugin and $reflect macro at all. That can be awesome indeed, it was hard to find this ts-macros plugin that outputs all the type info we need and it feels a bit tricky. Ok let's see if we can work out a separate build step in math5 to get the types available at runtime. It can indeed work out very neatly!

I'm having quite a good feeling about where math5 is heading.

> to me by far the most powerful argument is that math5 actually found typing violations in the implementation code that dispatcher_refactor did not. Agree, you're right, that is a strong argument in favor of `math5` for me too. (1, 7) Clear, thanks. In short: the builder pattern is a neccesity. That can be ok with me. How complex an API is I think is mostly how many concepts the user needs to userstand, needing to call a `.ship()` or `.build()` is not perse more complex (and a chained API is easier discoverable if you use an IDE with intellisense, so it explains itself). Not needing special constructs like "alias" really makes the API simpler, thats indeed great. (2, 3, 5, 6) Thanks, I now better understand how `math5` works and that it does not neccesary need a TypeScript plugin and `$reflect` macro at all. That can be awesome indeed, it was hard to find this `ts-macros` plugin that outputs all the type info we need and it feels a bit tricky. Ok let's see if we can work out a separate build step in `math5` to get the types available at runtime. It can indeed work out very neatly! I'm having quite a good feeling about where `math5` is heading.
josdejong commented 2024-10-07 16:06:14 +00:00
Author
Collaborator
Copy Link

Thoughts about a build step to generate types: in the current mathjs there is also a build step in place which generates files. In ./src/entry/ there will appear files named *.generated.js. It is handy to put the generated files inside ./src to make it work with the normal build tools, and it is handy to give them a distinctive extension like .generated.js to exclude them from commiting them to the source code.

Thoughts about a build step to generate types: in the current `mathjs` there is also a build step in place which generates files. In `./src/entry/` there will appear files named `*.generated.js`. It is handy to put the generated files inside `./src` to make it work with the normal build tools, and it is handy to give them a distinctive extension like `.generated.js` to exclude them from commiting them to the source code.
josdejong commented 2024-10-11 11:41:30 +00:00
Author
Collaborator
Copy Link

@glen I've pushed a branch feat/reflect-types where I experimented with a script ./tools/reflectTypes.mjs which injects type information from the .d.ts files into the corresponding .js files in the /build folder. If you now run pnpm go, it will replace all occurrences of .ship() with .ship({ reflectedType5: '...' }). In the .js files in /build you will now see occurrences of reflectedType5: ....

Note: you will only see reflectedType5 occurrences of the number files printed in the terminal and not those of the Complex files. That is because these are defined as functions that are not (yet) executed. You can look up the generated files in the /build folder yourself though to see them.

Thoughts/remarks:

  1. I haven't yet cleaned up the $reflect!(...) cases from the code but I think we can do that.
  2. A next step will be to parse and interpret the reflected types in such a form that it is usable by the Dispatcher (maybe a structured JSON object).
  3. I noticed that the .d.ts files still contains type information that we need to resolve, like inline cases of import("../Complex/type").Complex<number>. I haven't thought through how to solve that.
@glen I've pushed a branch [`feat/reflect-types`](https://code.studioinfinity.org/glen/math5/src/branch/feat/reflect-types) where I experimented with a script [`./tools/reflectTypes.mjs`](https://code.studioinfinity.org/glen/math5/src/branch/feat/reflect-types/tools/reflectTypes.mjs) which injects type information from the .d.ts files into the corresponding `.js` files in the `/build` folder. If you now run `pnpm go`, it will replace all occurrences of `.ship()` with `.ship({ reflectedType5: '...' })`. In the .js files in `/build` you will now see occurrences of `reflectedType5: ...`. Note: you will only see `reflectedType5` occurrences of the _number_ files printed in the terminal and not those of the _Complex_ files. That is because these are defined as functions that are not (yet) executed. You can look up the generated files in the `/build` folder yourself though to see them. Thoughts/remarks: 1. I haven't yet cleaned up the `$reflect!(...)` cases from the code but I think we can do that. 2. A next step will be to parse and interpret the reflected types in such a form that it is usable by the Dispatcher (maybe a structured JSON object). 3. I noticed that the .d.ts files still contains type information that we need to resolve, like inline cases of `import("../Complex/type").Complex<number>`. I haven't thought through how to solve that.
glen commented 2024-10-15 07:04:11 +00:00
Owner
Copy Link

@josdejong Thank you! It is nice to see a working type injector. The windows version of the build script didn't work on linux, so I tried a third option that I hope will work on both, and I removed the ts-macros since it seems, at least in this prototype, we won't be using it and I thought it would be good to simplify as much as possible. I took the liberty of rebasing your branch on these changes, and it still seems to work fine.

Since ultimately we will need some data structure for the types, rather than just a string (your point #2), I am going to make a further injection experiment branched from yours (I will call it feat/parse-reflection) in which I will use the TypeScript parser itself to parse the .d.ts files, along the lines of this StackOverflow

@josdejong Thank you! It is nice to see a working type injector. The windows version of the build script didn't work on linux, so I tried a third option that I hope will work on both, and I removed the ts-macros since it seems, at least in this prototype, we won't be using it and I thought it would be good to simplify as much as possible. I took the liberty of rebasing your branch on these changes, and it still seems to work fine. Since ultimately we will need some data structure for the types, rather than just a string (your point #2), I am going to make a further injection experiment branched from yours (I will call it feat/parse-reflection) in which I will use the TypeScript parser itself to parse the .d.ts files, along the lines of [this StackOverflow](https://stackoverflow.com/questions/39588436/how-to-parse-typescript-definition-to-json)
glen commented 2024-10-15 07:04:55 +00:00
Owner
Copy Link

P.S. Let me know if you get email notification of these comments; I think I have the code.studioinfinity.org mailer working again.

P.S. Let me know if you get email notification of these comments; I think I have the code.studioinfinity.org mailer working again.
josdejong commented 2024-10-15 07:55:28 +00:00
Author
Collaborator
Copy Link

Yes I'm getting email notifications again!

Good idea to try use the TypeScript parser itself to parse the .d.ts files, that would be nice. That will save us quite some work, then we only have to transform the TypeScript AST in something more tailored to what we need in the Dispatcher.

Yes I'm getting email notifications again! Good idea to try use the TypeScript parser itself to parse the .d.ts files, that would be nice. That will save us quite some work, then we only have to transform the TypeScript AST in something more tailored to what we need in the Dispatcher.
glen commented 2024-10-15 12:36:59 +00:00
Owner
Copy Link

All right, I have pushed a very preliminary parsed type injector on branch feat/parse-reflection. It is still very buggy/incomplete, but the basic structure of using the typescript parser and stripping the AST down to some simple-object specification of the type is there. I am not at all wedded to any of the particular representation choices, this is just a proof of concept. Anyhow, in the output of pnpm go (at the end of its reams of debugging text) you can see the structures that actually come out of the generated JavaScript and compare the reflectedType5 string property with the reflectedType6 structured property to see what you think. I will try to complete/polish it up a bit over the next week or so, and any time in the meantime if you have feedback/suggestions I am all ears.

All right, I have pushed a _very_ preliminary parsed type injector on branch feat/parse-reflection. It is still very buggy/incomplete, but the basic structure of using the typescript parser and stripping the AST down to some simple-object specification of the type is there. I am not at all wedded to any of the particular representation choices, this is just a proof of concept. Anyhow, in the output of `pnpm go` (at the end of its reams of debugging text) you can see the structures that actually come out of the generated JavaScript and compare the reflectedType5 string property with the reflectedType6 structured property to see what you think. I will try to complete/polish it up a bit over the next week or so, and any time in the meantime if you have feedback/suggestions I am all ears.
glen commented 2024-10-18 05:45:02 +00:00
Owner
Copy Link

OK, now the parsed type injector covers all of the factories/implementations exported by any module in the current math5 (this is on the feat/parse-reflection branch). There are still some unhandled AST node types, but they are not used in the export of any particular factories/implementations.

So I thought of this as a good time to take stock. Please let me know your thoughts on:

  1. How you like the ._reflectedType6 properties as compared to reflectedType5? I feel one advantage is that they show up even for the template functions in Complex, without having to call the function.
  2. Your thoughts on injecting them as properties on the symbols (as reflectedType6 does) vs. arguments to the ship() function (as reflectedType5 does). Yet a third possibility would be for an identifier foo to export the type information in a separate value with a conventional name foo$type. I wouldn't even entertain that possibility, as being very ad-hoc, except that it's the only mechanism I can think of that will allow us to export reflection information about specific type entities that are not attached to any exported values. For example, we may need to know type information about (say) Complex<T>. But Complex is just a type in typescript; there is no corresponding value in the generated JavaScript. Hence there is no .ship() call that ships it, nor even a value Complex to attach a property to. But we could export the type information in a value with a conventional name, like Complex$type (from which we could learn that a Complex has two fields, re and im, among other things). This is actually more important for us for things like at runtime looking up what the RealType or the ZeroType of a given type is.
  3. Your thoughts on whether we should generate type reflection information for all entities, or just the shipped implementations/factories (I doubt that's enough because of needing at runtime to know RealTypes etc.), or somewhere in between.
  4. Your thoughts on the notation for types that's being generated. It's pretty verbose, but it's also pretty much just a direct translation of the typescript AST for a type into a jsonlike representation. There are a lot of possibilities for what a TypeScript type can be -- it's a pretty intricate type system. Do you know of any jsonish notation for TypeScript types that we could just adopt rather than roll our own like this branch is currently doing? A very cursory web search didn't turn up anything for me.

When we've sort of converged on these points, I think we can wrap up these branches into a PR and move on to implementing a Dispatcher :-)

OK, now the parsed type injector covers all of the factories/implementations exported by any module in the current math5 (this is on the feat/parse-reflection branch). There are still some unhandled AST node types, but they are not used in the export of any particular factories/implementations. So I thought of this as a good time to take stock. Please let me know your thoughts on: 1. How you like the ._reflectedType6 properties as compared to reflectedType5? I feel one advantage is that they show up even for the template functions in Complex, without having to call the function. 2. Your thoughts on injecting them as properties on the symbols (as reflectedType6 does) vs. arguments to the ship() function (as reflectedType5 does). Yet a third possibility would be for an identifier foo to export the type information in a separate value with a conventional name foo$type. I wouldn't even entertain that possibility, as being very ad-hoc, except that it's the only mechanism I can think of that will allow us to export reflection information about specific _type_ entities that are not attached to any exported values. For example, we may need to know type information about (say) `Complex<T>`. But Complex is just a type in typescript; there is no corresponding _value_ in the generated JavaScript. Hence there is no `.ship()` call that ships it, nor even a value `Complex` to attach a property to. But we _could_ export the type information in a value with a conventional name, like `Complex$type` (from which we could learn that a Complex has two fields, re and im, among other things). This is actually more important for us for things like at runtime looking up what the RealType or the ZeroType of a given type is. 3. Your thoughts on whether we should generate type reflection information for all entities, or just the shipped implementations/factories (I doubt that's enough because of needing at runtime to know RealTypes etc.), or somewhere in between. 4. Your thoughts on the notation for types that's being generated. It's pretty verbose, but it's also pretty much just a direct translation of the typescript AST for a type into a jsonlike representation. There are a lot of possibilities for what a TypeScript type can be -- it's a pretty intricate type system. Do you know of any jsonish notation for TypeScript types that we could just adopt rather than roll our own like this branch is currently doing? A very cursory web search didn't turn up anything for me. When we've sort of converged on these points, I think we can wrap up these branches into a PR and move on to implementing a Dispatcher :-)
josdejong commented 2024-10-18 08:53:04 +00:00
Author
Collaborator
Copy Link

That sounds promising! I've checked out feat/parse-reflection but I see the logging like:

Defs from C:\Users\wjosd\projects\misc\math5\build\numbers\arithmetic.d.ts are undefined

For some reason ts2json(defFile) always returns undefined on my machine 🤔. I'll try with a linux terminal (running Windows + nodejs 20 or 22 normally).

That sounds promising! I've checked out `feat/parse-reflection` but I see the logging like: ``` Defs from C:\Users\wjosd\projects\misc\math5\build\numbers\arithmetic.d.ts are undefined ``` For some reason `ts2json(defFile)` always returns `undefined` on my machine 🤔. I'll try with a linux terminal (running Windows + nodejs 20 or 22 normally).
josdejong commented 2024-10-18 08:58:25 +00:00
Author
Collaborator
Copy Link

O wow, works in Ubuntu via WSL. That is interesting.

O wow, works in Ubuntu via WSL. That is interesting.
josdejong commented 2024-10-18 09:55:46 +00:00
Author
Collaborator
Copy Link

This math5 approach looks more and more promising Glen! Awesome. The separate build step is working out quite nicely.

  1. I love the _reflectedType6 JSON structure! This should be easy to use in the Dispatcher.

  2. What is most important to me is that (a) it is easy and reliable to access the correct reflectedType6 from anything that is imported into the Dispatcher, also when people do creative things with import, export, putting stuff inside objects, returning stuff from functions etc. And (b) that the way we inject the types cannot interfere with other users stuff. My first thought was that attaching the type information to .ship() would be the most straightforward and reliable way to do that, but I'm totally fine with other solutions if that works fine. One thing that feels inconsistent to me is that mixed._reflectedType6 is attached to the function mixed but I think it should be attached to the return value of the function instead. That return value is not explicitely defined as a variable though, so that is not possible. What if people go wild and do dynamic things like the following example? I have the feeling that attaching the type information to .ship() is more robust, since that is guarenteed to be attached to the right chain of implementations.

    export function conditional(someCondition: boolean) {
  if (someCondition) {
    // some set of functions
    return implementations().dependent({ add: multiplyNumber, ... }).ship()
  } else {
    // a different set of functions
    return implementations().dependent({ add: multiplyGeneric, ... }).ship()
  }
}
// where to attach type information in this case?

    About Complex type vs actual implementation: We will need define the name and implementation of data types like Complex in the Dispatcher somehow, right? Something like implementations().defineType('Complex', Complex) or implementations().defineTypes({ 'Complex': Complex })?

  3. Ideally we ship as little data as possible I guess, we should only use the information we need. I think we take the easy way for now, and optimize this once we have a basic POC up and running, so we have a better understanding of it?

  4. I like the clear structure! We can see if there are ways to make the JSON structure more predictable, like parameters always being an object (right now it can be "number" but also {"_typeParameter": "T"} for example). I think we can better see any opportunities/inconsistencies by defining TypeScript types for the generated JSON structure holding the reflected types.

    In the end we have to check the impact of all these data structures on the bundle size, since JSON is not very efficient. But there are ways to solve that, I think we can best look into that in a later stage.

  5. Some thoughts on naming: is there a special reason to start all keys with an underscore, like "_parameters" and "_returns"? I associate that with unused parameters or private properties. Also, shall we rename .ship() to .done() or .build()?

  6. I have to figure out why this isn't working on Windows. Any ideas on where to look? Does undici-types need to be registered somehow like npx ts-patch install?

  7. What are the undici-types needed for?

This `math5` approach looks more and more promising Glen! Awesome. The separate build step is working out quite nicely. 1. I love the `_reflectedType6` JSON structure! This should be easy to use in the Dispatcher. 2. What is most important to me is that (a) it is easy and reliable to access the correct `reflectedType6` from anything that is imported into the Dispatcher, also when people do creative things with import, export, putting stuff inside objects, returning stuff from functions etc. And (b) that the way we inject the types cannot interfere with other users stuff. My first thought was that attaching the type information to `.ship()` would be the most straightforward and reliable way to do that, but I'm totally fine with other solutions if that works fine. One thing that feels inconsistent to me is that `mixed._reflectedType6` is attached to the function `mixed` but I think it should be attached to the return value of the function instead. That return value is not explicitely defined as a variable though, so that is not possible. What if people go wild and do dynamic things like the following example? I have the feeling that attaching the type information to `.ship()` is more robust, since that is guarenteed to be attached to the right chain of implementations. ```js export function conditional(someCondition: boolean) { if (someCondition) { // some set of functions return implementations().dependent({ add: multiplyNumber, ... }).ship() } else { // a different set of functions return implementations().dependent({ add: multiplyGeneric, ... }).ship() } } // where to attach type information in this case? ``` About `Complex` type vs actual implementation: We will need define the name and implementation of data types like `Complex` in the Dispatcher somehow, right? Something like `implementations().defineType('Complex', Complex)` or `implementations().defineTypes({ 'Complex': Complex })`? 3. Ideally we ship as little data as possible I guess, we should only use the information we need. I think we take the easy way for now, and optimize this once we have a basic POC up and running, so we have a better understanding of it? 4. I like the clear structure! We can see if there are ways to make the JSON structure more predictable, like parameters always being an object (right now it can be `"number"` but also `{"_typeParameter": "T"}` for example). I think we can better see any opportunities/inconsistencies by defining TypeScript types for the generated JSON structure holding the reflected types. In the end we have to check the impact of all these data structures on the bundle size, since JSON is not very efficient. But there are ways to solve that, I think we can best look into that in a later stage. 5. Some thoughts on naming: is there a special reason to start all keys with an underscore, like "_parameters" and "_returns"? I associate that with unused parameters or private properties. Also, shall we rename `.ship()` to `.done()` or `.build()`? 6. I have to figure out why this isn't working on Windows. Any ideas on where to look? Does `undici-types` need to be registered somehow like `npx ts-patch install`? 7. What are the `undici-types` needed for?
glen commented 2024-10-18 17:58:16 +00:00
Owner
Copy Link

Wow, a lot of great discussion. As before, I will mostly reply to each item in a separate post rather than create a mega one.

  1. Glad!

  2. (a) "Easy and reliable to access types for anything that is imported into dispatcher" -- always having Dispatcher look on a specific property like _reflected_type or whatever seems easy and reliable. Then of course we would have to make sure it is always there. If we attach the information via .ship(), that creates a requirement to always use ship(). But the (only) point of implemenations()... .ship() is to look up standard signatures and thereby free you up from having to reiterate types in your implementations/dependencies. So I had imagined that a one-off function that really just had a single signature, like for argument's sake factorial that can take a number or bigint and always returns a bigint (since it grows so fast), we would like to be able to just

export const factorial = {
   implementation:  (n: number | bigint): bigint => { code goes here }
}

and then that could be loaded directly into the Dispatcher (and we wouldn't even need to put factorial in CommonSignature, etc. So the point is that for one-off functions, it's actually more DRY not to use ship). For that to work, the types of factorial have to be reflected somewhere accessible., but there is no ship() call here.

So the ts2json.mjs tool I've been developing is intended to just do type reflection of every symbol that's exported, that way it's always there for the Dispatcher if it needs it.

(b) "type injection can't interfere" -- I agree that this is a strength of injecting into ship(), and might even be an argument to absolutely require ship() to work with dispatcher, if that doesn't seem too cumbersome to require across the board. With the ts2json.mjs method, we have to pick a property that is "unlikely" that others will use, and then let people know not to use it -- or, actually, perhaps we could create a Symbol and put the reflected types on the property named by that Symbol, to prevent any chance of conflict. Do you think that would work?

On where to attach type information to mixed() -- it depends on what is going to be fed into the dispatcher to specify those implementations. The Dispatcher has to know that those implementations are generic. In TypeScript, you have to call mixed with a specific type for T, say number to get Complex<number> operations, but then the returned item is no longer generic -- it's specialized to provide Complex<number> implementations for add/divide. I recognize that from the point of view of the object JavaScript code, there is no distinction (it returns precisely the same JavaScript regardless of what type mixed() is called with), but conversely at the JavaScript layer, there are no types either.

The upshot of these thoughts is that it made more conceptual sense to me to pass the mixed() function itself to the dispatcher, and then when it needs to instantiate Gaussian integers Complex<bigint>, say, it will call mixed(). From that point of view, the annotation should be on the mixed() function, which aligns nicely with the fact that these are generic implementations, and that mixed() function is a generic function. I do realize that there is an alternate world in which something calls mixed(), and even though that result is now in some sense concrete rather than generic, since there is no difference at the JavaScript level, it just marks that result with generic type descriptions, and then Dispatcher can use the results when it needs to instantiate any particular Complex<SomeType>. But the other obstacle to this alternate view is what would that thing be that calls mixed()? Why wouldn't that be the Dispatcher? The convenience of just importing a bunch of implementation modules and then just dumping whatever they export into the Dispatcher seems really attractive, in which case mixed would get sent to the Dispatcher as a function, and it would be responsible for calling mixed to get the implementations (when it saw fit), in which case the type annotation should be on mixed, not on what it returns, so that the Dispatcher knows what to do with this 'mixed' thing it got. If there's no annotation at the level of mixed, it wouldn't be able to know what to do with it...

Your example of a conditional export of two different groups of implementations is an interesting and challenging one. Let's say something like this is useful and should be supported. We should recognize that with either the injecting into ship() or injecting as a property on exported symbols, these latest two approaches only have access to the type information that is recorded in the .d.ts files (that is one argument in favor of going back to ts-macros -- although in the experiment they were producing less detailed type information, at least there we have the potential to enhance them and get access to finer-grained type information, like the separate type of each branch of an if-statement, that is lost at the top level .d.ts files).

OK, so supposing we are pursuing a .d.ts-based solution. Then all we get in the .d.ts file is the type of the function conditional. It doesn't record separate types for inner blocks of the code for conditional (although of course that type information is generated during the compilation, which is why something like ts-macros might be necessary if we want to try to capture that information). And what is the type of conditional? It is something like

boolean => ({add: NumberSig, ...} | {add: GenericSig, ...})

So in the ship()-injection solution, the injector will have to look at the code for conditional, see that there are multiple instances of ship() inside its code, and then guess that that they correspond to the different branches of the union return type of the type of conditional, and hope that they are listed in the same order as the different ship() calls in conditional, and then inject them. If it gets all these guesses right, then whatever calls conditional() will have an easy time: it will get an object with appropriate type reflection.

In the add-a-property-to-exports solution, the conditional function itself will be labeled with the above type (but in JSON form). The code for Dispatcher to know when to even call this function will be a bit confusing -- if it wants an implementation in either listed collection, it should just go ahead and try calling it, and hope it get back what it needs? Will it be able to check? How will it know what arguments to call conditional with, and where will it get them?

So I do kind of agree that the only path that has much of a chance of being able to do something with an implementation supplier like conditional in a .d.ts-only scheme is something like ship()-injection, but even with such a plan it is not clear to me (I) who will be responsible for calling conditional() and where it will get the arguments from; (II) what the use-case for this facility would be; (III) that we could reliably do the chopping of the return type into separate type specifications and injection of each piece into the proper ship() call (or that we want to try to code this). So to me it's unclear that we want to base our decision on this conditional() example; but I trust your experience and intuition, so if you think we need to plan to support something like conditional() then we should pursue ship()-injection. Note we can use the type objects generated by ts2json.mjs in a ship()-injection construction; we just need to analyze the .js files as well as the .d.ts files (I had been hoping to eliminate all analysis of the .js output of the first (tsc) build step since I fear just naive regexp-matching ultimately won't be enough, and it would be nice not to have to pull in a js parser as well, although they certainly exist.)

Finally: "We will need define the name and implementation of data types like Complex in the Dispatcher somehow, right?"
Yes, the concept in the current prototype is that there are objects Complex_type and number_type exported by the respective modules, and these exports are dumped into Dispatcher, and as it is combing through everything it got and assembling it into a collection of actually callable functions, it recognizes these objects as adding data types rather than as supplying implementations, either because of the _type suffix of their names or because of the collection of properties that they have (agnostic at the moment as to which). But anyhow, to me that's why it's important that these type-designation objects also have reflected type information. In the "attach a property" scheme they will; in the ship() scheme I think we will need to add a .addType() method alongside .independent() and .dependent() that can come before ship() so that they are included in the type information that gets generated.

OK, I think that's everything raised in your item (2) -- it's kind of big bundle!

Wow, a lot of great discussion. As before, I will mostly reply to each item in a separate post rather than create a mega one. 1. Glad! 2. (a) "Easy and reliable to access types for anything that is imported into dispatcher" -- always having Dispatcher look on a specific property like `_reflected_type` or whatever seems easy and reliable. Then of course we would have to make sure it is always there. If we attach the information via .ship(), that creates a requirement to always use ship(). But the (only) point of `implemenations()... .ship()` is to look up standard signatures and thereby free you up from having to reiterate types in your implementations/dependencies. So I had imagined that a one-off function that really just had a single signature, like for argument's sake factorial that can take a number or bigint and always returns a bigint (since it grows so fast), we would like to be able to just ``` export const factorial = { implementation: (n: number | bigint): bigint => { code goes here } } ``` and then that could be loaded directly into the Dispatcher (and we wouldn't even need to put factorial in CommonSignature, etc. So the point is that for one-off functions, it's actually more DRY not to use ship). For that to work, the types of factorial have to be reflected somewhere accessible., but there is no ship() call here. So the ts2json.mjs tool I've been developing is intended to just do type reflection of every symbol that's exported, that way it's always there for the Dispatcher if it needs it. (b) "type injection can't interfere" -- I agree that this is a strength of injecting into ship(), and might even be an argument to absolutely require ship() to work with dispatcher, if that doesn't seem too cumbersome to require across the board. With the ts2json.mjs method, we have to pick a property that is "unlikely" that others will use, and then let people know not to use it -- or, actually, perhaps we could create a Symbol and put the reflected types on the property named by that Symbol, to prevent any chance of conflict. Do you think that would work? On where to attach type information to mixed() -- it depends on what is going to be fed into the dispatcher to specify those implementations. The Dispatcher has to know that those implementations are generic. In TypeScript, you have to call mixed with a specific type for T, say `number` to get `Complex<number>` operations, but then the returned item is no longer generic -- it's specialized to provide `Complex<number>` implementations for add/divide. I recognize that from the point of view of the object JavaScript code, there is no distinction (it returns precisely the same JavaScript regardless of what type mixed() is called with), but conversely at the JavaScript layer, there are no types either. The upshot of these thoughts is that it made more conceptual sense to me to pass the mixed() function itself to the dispatcher, and then when it needs to instantiate Gaussian integers `Complex<bigint>`, say, it will call mixed(). From that point of view, the annotation should be on the mixed() function, which aligns nicely with the fact that these are generic implementations, and that mixed() function is a generic function. I do realize that there is an alternate world in which something calls mixed(), and even though that result is now in some sense concrete rather than generic, since there is no difference at the JavaScript level, it just marks that result with generic type descriptions, and then Dispatcher can use the results when it needs to instantiate any particular `Complex<SomeType>`. But the other obstacle to this alternate view is what would that thing be that calls mixed()? Why wouldn't that be the Dispatcher? The convenience of just importing a bunch of implementation modules and then just dumping whatever they export into the Dispatcher seems really attractive, in which case mixed would get sent to the Dispatcher as a function, and it would be responsible for calling mixed to get the implementations (when it saw fit), in which case the type annotation should be on mixed, not on what it returns, so that the Dispatcher knows what to do with this 'mixed' thing it got. If there's no annotation at the level of mixed, it wouldn't be able to know what to do with it... Your example of a conditional export of two different groups of implementations is an interesting and challenging one. Let's say something like this is useful and should be supported. We should recognize that with either the injecting into ship() or injecting as a property on exported symbols, these latest two approaches only have access to the type information that is recorded in the .d.ts files (that is one argument in favor of going back to ts-macros -- although in the experiment they were producing less detailed type information, at least there we have the potential to enhance them and get access to finer-grained type information, like the separate type of each branch of an if-statement, that is lost at the top level .d.ts files). OK, so supposing we are pursuing a .d.ts-based solution. Then all we get in the .d.ts file is the type of the function `conditional`. It doesn't record separate types for inner blocks of the code for `conditional` (although of course that type information is generated during the compilation, which is why something like ts-macros might be necessary if we want to try to capture that information). And what is the type of `conditional`? It is something like `boolean => ({add: NumberSig, ...} | {add: GenericSig, ...})` So in the ship()-injection solution, the injector will have to look at the code for `conditional`, see that there are multiple instances of ship() inside its code, and then guess that that they correspond to the different branches of the union return type of the type of `conditional`, and hope that they are listed in the same order as the different ship() calls in `conditional`, and then inject them. If it gets all these guesses right, then whatever calls `conditional()` will have an easy time: it will get an object with appropriate type reflection. In the add-a-property-to-exports solution, the conditional function itself will be labeled with the above type (but in JSON form). The code for Dispatcher to know when to even call this function will be a bit confusing -- if it wants an implementation in either listed collection, it should just go ahead and try calling it, and hope it get back what it needs? Will it be able to check? How will it know what arguments to call `conditional` with, and where will it get them? So I do kind of agree that the only path that has much of a chance of being able to do something with an implementation supplier like `conditional` in a .d.ts-only scheme is something like ship()-injection, but even with such a plan it is not clear to me (I) who will be responsible for calling conditional() and where it will get the arguments from; (II) what the use-case for this facility would be; (III) that we could reliably do the chopping of the return type into separate type specifications and injection of each piece into the proper ship() call (or that we want to try to code this). So to me it's unclear that we want to base our decision on this `conditional()` example; but I trust your experience and intuition, so if you think we need to plan to support something like `conditional()` then we should pursue ship()-injection. Note we can use the type objects generated by ts2json.mjs in a ship()-injection construction; we just need to analyze the .js files as well as the .d.ts files (I had been hoping to eliminate all analysis of the .js output of the first (tsc) build step since I fear just naive regexp-matching ultimately won't be enough, and it would be nice not to have to pull in a js parser as well, although they certainly exist.) Finally: "We will need define the name and implementation of data types like Complex in the Dispatcher somehow, right?" Yes, the concept in the current prototype is that there are objects Complex_type and number_type exported by the respective modules, and these exports are dumped into Dispatcher, and as it is combing through everything it got and assembling it into a collection of actually callable functions, it recognizes these objects as adding data types rather than as supplying implementations, either because of the _type suffix of their names or because of the collection of properties that they have (agnostic at the moment as to which). But anyhow, to me that's why it's important that these type-designation objects also have reflected type information. In the "attach a property" scheme they will; in the ship() scheme I think we will need to add a .addType() method alongside .independent() and .dependent() that can come before ship() so that they are included in the type information that gets generated. OK, I think that's everything raised in your item (2) -- it's kind of big bundle!
glen commented 2024-10-18 18:05:31 +00:00
Owner
Copy Link
  1. Ideally we ship as little data as possible I guess, we should only use the information we need. I think we take the easy way for now, and optimize this once we have a basic POC up and running

Right, although the "easy way" is different for add-a-property and ship()-injection; in the former, it is easiest just to add the property to all exports, whereas in ship()-injection the only thing one can do is analyze the types that have been found to determine which correspond to ship() calls and then find the proper ship() calls in the .js code to insert them into. (I.e., I feel ship()-injection is more work at the second-build-step level for sure.)

> 3. Ideally we ship as little data as possible I guess, we should only use the information we need. I think we take the easy way for now, and optimize this once we have a basic POC up and running Right, although the "easy way" is different for add-a-property and ship()-injection; in the former, it is easiest just to add the property to all exports, whereas in ship()-injection the only thing one can do is analyze the types that have been found to determine which correspond to ship() calls and then find the proper ship() calls in the .js code to insert them into. (I.e., I feel ship()-injection is more work at the second-build-step level for sure.)
glen commented 2024-10-18 18:16:28 +00:00
Owner
Copy Link
  1. We can see if there are ways to make the JSON structure more predictable, like parameters always being an object (right now it can be "number" but also {"_typeParameter": "T"} for example).

Sure, we could write {_intrinsic: 'number'} if you prefer that to just 'number'. Let me know if you want me to make that change.

I think we can better see any opportunities/inconsistencies by defining TypeScript types for the generated JSON structure holding the reflected types.

Yes, I suppose that the Dispatcher will be written in TypeScript (and just use vague types for the implementations) and will need to do that.

You don't know of any other specification of the TypeScript type system in a JSON-like notation out there?

> 4. We can see if there are ways to make the JSON structure more predictable, like parameters always being an object (right now it can be "number" but also {"_typeParameter": "T"} for example). Sure, we could write `{_intrinsic: 'number'}` if you prefer that to just `'number'`. Let me know if you want me to make that change. > I think we can better see any opportunities/inconsistencies by defining TypeScript types for the generated JSON structure holding the reflected types. Yes, I suppose that the Dispatcher will be written in TypeScript (and just use vague types for the implementations) and will need to do that. You don't know of any other specification of the TypeScript type system in a JSON-like notation out there?
glen commented 2024-10-18 18:30:49 +00:00
Owner
Copy Link
  1. Is there a special reason to start all keys with an underscore, like "_parameters" and "_returns"? I associate that with unused parameters or private properties.

Well, I do feel like the type information is sort of private to Dispatcher, not really intended for general purpose use. But more importantly, currently the type of a plain object like {x: 1.0, y: -2.5, z: 0.1} is encoded as {x: 'number', y: 'number', z: 'number'} so the underscores are to distinguish the type-specification metaproperties from just regular properties that are part of a particular type, not to mention the names of operations that the Dispatcher will collect. (And we will have to say that to use Dispatcher, your types can't have properties that start with _, nor can you use operation names that start with _, or we will have to reserve some other special spelling for our type-specification metaproperties, or, like I suggested for the property on which to put the reflected type itself, we could try switching to a collection of Symbols that can't conflict with property names or operation names or even other symbols (because no two invocations can ever make the same Symbol). I think that would work -- would you prefer switching to that?

Also, shall we rename .ship() to .done() or .build()?

Yeah, I agree ship() isn't a super name. What would make sense to me is to rename both implementations and ship to things that conceptually "match" like braces or if and endif for languages that go for that sort of thing. So maybe, since it's all for the dispatcher anyway, dispatcherGroup() and endGroup()? Or if you prefer implementationGroup() and endGroup()? or startImplementations() and endImplementations()? Really anything that reads like a matching pair would be fine with me...

> 5. Is there a special reason to start all keys with an underscore, like "_parameters" and "_returns"? I associate that with unused parameters or private properties. Well, I do feel like the type information is sort of private to Dispatcher, not really intended for general purpose use. But more importantly, currently the type of a plain object like `{x: 1.0, y: -2.5, z: 0.1}` is encoded as `{x: 'number', y: 'number', z: 'number'}` so the underscores are to distinguish the type-specification metaproperties from just regular properties that are part of a particular type, not to mention the names of operations that the Dispatcher will collect. (And we will have to say that to use Dispatcher, your types can't have properties that start with `_`, nor can you use operation names that start with `_`, or we will have to reserve some other special spelling for our type-specification metaproperties, or, like I suggested for the property on which to put the reflected type itself, we could try switching to a collection of Symbols that can't conflict with property names or operation names or even other symbols (because no two invocations can ever make the same Symbol). I think that would work -- would you prefer switching to that? > Also, shall we rename .ship() to .done() or .build()? Yeah, I agree `ship()` isn't a super name. What would make sense to me is to rename both `implementations` and `ship` to things that conceptually "match" like braces or `if` and `endif` for languages that go for that sort of thing. So maybe, since it's all for the dispatcher anyway, `dispatcherGroup()` and `endGroup()`? Or if you prefer `implementationGroup()` and `endGroup()`? or `startImplementations()` and `endImplementations()`? Really anything that reads like a matching pair would be fine with me...
glen commented 2024-10-18 18:33:13 +00:00
Owner
Copy Link
  1. I have to figure out why this isn't working on Windows. Any ideas on where to look?

Not really. I can access Windows to try to debug if it becomes really critical, but I have never even cloned a git repo on a Windows machine, so I would literally be starting from zero.

Does undici-types need to be registered somehow like npx ts-patch install?

I am only doing "npm install" when I clone the math5 repo, so I don't think anything beyond that should be needed.

> 6. I have to figure out why this isn't working on Windows. Any ideas on where to look? Not really. I can access Windows to try to debug if it becomes really critical, but I have never even cloned a git repo on a Windows machine, so I would literally be starting from zero. > Does undici-types need to be registered somehow like npx ts-patch install? I am only doing "npm install" when I clone the math5 repo, so I don't think anything beyond that should be needed.
glen commented 2024-10-18 18:37:56 +00:00
Owner
Copy Link
  1. What are the undici-types needed for?

Well, the answer is either "I don't know" or "they shouldn't be", but the fact is without them added, the whole shebang does not work. I am certainly not using them directly. I only added them because of errors/failures I was getting, and then because of finding issues like https://github.com/DefinitelyTyped/DefinitelyTyped/discussions/67406 when I was trying to debug the (very mysterious) situation. I think it is an unresolved problem with @types/node, unless I am misunderstanding. In any case, since this was a POC, I wasn't too concerned, I just wanted everything to run.

> 7. What are the undici-types needed for? Well, the answer is either "I don't know" or "they shouldn't be", but the fact is without them added, the whole shebang does not work. I am certainly not using them directly. I only added them because of errors/failures I was getting, and then because of finding issues like https://github.com/DefinitelyTyped/DefinitelyTyped/discussions/67406 when I was trying to debug the (very mysterious) situation. I _think_ it is an unresolved problem with `@types/node`, unless I am misunderstanding. In any case, since this was a POC, I wasn't _too_ concerned, I just wanted everything to run.
glen commented 2024-10-18 18:42:09 +00:00
Owner
Copy Link

OK, I think I've covered everything you've raised. When you've had a chance to digest it, let me know your thoughts on what immediate direction to go and then I/we can try to bundle up these branches into a proper PR into main() (with only one set of reflected type information, etc.).

OK, I think I've covered everything you've raised. When you've had a chance to digest it, let me know your thoughts on what immediate direction to go and then I/we can try to bundle up these branches into a proper PR into main() (with only one set of reflected type information, etc.).
josdejong commented 2024-10-21 09:29:41 +00:00
Author
Collaborator
Copy Link

Ha ha, you managed to produce a "mega post" for point 2 alone 😉

  1. 👍

  2. (b) In the past, I have avoided using Symbol for such cases and used a "unique enough" property because Symbols where harder to debug and cannot be serialized. I have a slight preference for using a "unique enough" property name over Symbols to keep things simple, even with the theoretic possibility of a naming conflict. If you have a strong preference though I'm totally fine with trying out Symbol to see if it works out nicely.

    When zooming out: the automatic type reflection will have certain limitations, we will have to document that. Like: maybe we should not try to support dynamic conditionals if we attach the type information as property to the root objects/functions in all files.

    I think we're on the same page in wanting a solution with as little verbosity for the user as possible, and not needing .ship() in may cases sounds very good :).

    I propose the following: let's not try to think through all and every (theoretic) edge cases upfront. Let's go for attaching as a property (and not injecting in .ship(...)) for now, built a first version of the Dispatcher, and see how it works out. It probably works out nicely, but if not, it will not be that much work to rewrite it to a different solution like injecting inside .ship(...). But at that point we will have a more clear view on the actual limitations and edge cases and can easier see the pros/cons. Ok?

  3. 👍

  4. Let's not change the JSON structure upfront, but see how it works out when we define the types as JSON and implement the Dispatcher, ok? Only make changes if we see concrete room for improvement.

    I don't know of any existing JSON-like notations, and also, I expect we want to write it in such a way that it is aligned to exactly what the Dispatcher needs. Let's just roll our own.

  5. Thanks, I understand the reason for using _ to distinguish the type information structure itself from the data. I expect there is no real need for it when the data structure of the type information is explicitely defined, but it may be of help when reading and debugging it ourselves. Like with (4) I propose we keep it as it is for now (with underscores). And at some point I may try out refactoring it to see how an alterative without underscores works out.

  6. good to know for sure that there are no special installation steps needed. I'll first do more debugging on my own, and try search on internet for this kind of issue. It is a critical thing in the end to have this work on any machine (Windows/Linux/Mac). But I assume it is solvable.

  7. got it, thanks. Maybe the Windows issue (6) has to do with this too, that's why I asked.

Ha ha, you managed to produce a "mega post" for point 2 alone 😉 1. 👍 2. (b) In the past, I have avoided using Symbol for such cases and used a "unique enough" property because Symbols where harder to debug and cannot be serialized. I have a slight preference for using a "unique enough" property name over Symbols to keep things simple, even with the theoretic possibility of a naming conflict. If you have a strong preference though I'm totally fine with trying out Symbol to see if it works out nicely. When zooming out: the automatic type reflection _will_ have certain limitations, we will have to document that. Like: maybe we should not try to support dynamic conditionals if we attach the type information as property to the root objects/functions in all files. I think we're on the same page in wanting a solution with as little verbosity for the user as possible, and not needing `.ship()` in may cases sounds very good :). I propose the following: let's not try to think through all and every (theoretic) edge cases upfront. Let's go for attaching as a property (and not injecting in `.ship(...)`) for now, built a first version of the Dispatcher, and see how it works out. It probably works out nicely, but if not, it will not be _that_ much work to rewrite it to a different solution like injecting inside `.ship(...)`. But at that point we will have a more clear view on the actual limitations and edge cases and can easier see the pros/cons. Ok? 3. 👍 4. Let's not change the JSON structure upfront, but see how it works out when we define the types as JSON and implement the Dispatcher, ok? Only make changes if we see concrete room for improvement. I don't know of any existing JSON-like notations, and also, I expect we want to write it in such a way that it is aligned to exactly what the Dispatcher needs. Let's just roll our own. 5. Thanks, I understand the reason for using _ to distinguish the type information structure itself from the data. I expect there is no real need for it when the data structure of the type information is explicitely defined, but it may be of help when reading and debugging it ourselves. Like with (4) I propose we keep it as it is for now (with underscores). And at some point I may try out refactoring it to see how an alterative without underscores works out. 6. good to know for sure that there are no special installation steps needed. I'll first do more debugging on my own, and try search on internet for this kind of issue. It is a critical thing in the end to have this work on any machine (Windows/Linux/Mac). But I assume it is solvable. 7. got it, thanks. Maybe the Windows issue (6) has to do with this too, that's why I asked.
josdejong commented 2024-10-21 09:35:55 +00:00
Author
Collaborator
Copy Link

So, in short: let's keep the POC as it is right now, built a first version of the dispatcher, and after that see if there is need to finetune the API's.

(5) O: and about the naming of the methods: I don't think it's really needed to try have a matching "start" and "end" name since the builder pattern is quite a common and well known one. Some more ideas:

  • dispatch() ... .done()
  • define() ... .done()
  • implementations() ... .done()
  • create() ... .done()
  • build() ... .done()
So, in short: let's keep the POC as it is right now, built a first version of the dispatcher, and after that see if there is need to finetune the API's. (5) O: and about the naming of the methods: I don't think it's really needed to try have a matching "start" and "end" name since the builder pattern is quite a common and well known one. Some more ideas: - `dispatch() ... .done()` - `define() ... .done()` - `implementations() ... .done()` - `create() ... .done()` - `build() ... .done()`
josdejong commented 2024-10-21 18:23:21 +00:00
Author
Collaborator
Copy Link

Ok the Windows issue is solved 😁, I've pushed a commit in main: 2a8823c5f7

Ok the Windows issue is solved 😁, I've pushed a commit in `main`: https://code.studioinfinity.org/glen/math5/commit/2a8823c5f7b75774980a7c5ddd2db8bbcaa57880
glen commented 2024-10-21 22:39:51 +00:00
Owner
Copy Link

OK, I think we're set to write a dispatcher. Should this issue be closed?

OK, I think we're set to write a dispatcher. Should this issue be closed?
josdejong commented 2024-10-22 07:15:24 +00:00
Author
Collaborator
Copy Link

Yes let's close this issue. I'm looking forward to the next step :)

Yes let's close this issue. I'm looking forward to the next step :)
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: glen/math5#1
No description provided.
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?