Fixes #1305 - Convert constants to enums #1389
Conversation
…eding to use constants
|
I think this is a good point to discuss how obsolescence should be handled going forward. C# supports an [Obsolete] attribute, I assume there's nothing like that in JS, but there might be in TS? Besides doing this in code, it can be handled in a predictable way and announced in build release notes. Perhaps something like the following would work:
@JoshEngebretson just pulling you into this convo :) |
|
I searched for some kind of [Obsolete] annotation for TypeScript as that's really handy in C#, but unfortunately there is nothing really available yet. There has been talk about how to handle it though. In the mean time, I've just added comments on those elements that will get picked up by TypeDoc. I like the idea of creating followup tasks to remove deprecated items. That would be much better than scouring the code for |
|
Yeah I think we all know how often those actually lead to things being done >.> |
|
There is some talk about handling deprecations and obsolutions in TypeScript here. But not currently a thing. |
|
@shaddockh Thanks for the PR, I'll check it out a bit later, looks good from a quick glance. Yeah, C# compiler/IDE's have good support for deprecation... on TS, would be nice, though I think we can survive for a bit, for now good to have the comments in place for future TS deprecation language feature. |
| else if (name == "BT_DYNAMIC") | ||
| value = "2"; | ||
| } | ||
|
|
There was a problem hiding this comment.
Nice cleanup :) This is something that would be nice to be able to do in C#, though we can't due to constant requirements
| { | ||
| String name = (*itr).first_; | ||
| source.AppendWithFormat("duk_push_number(ctx, (double) %s);\n", name.CString()); | ||
| source.AppendWithFormat("duk_put_prop_string(ctx, -2, \"%s\");\n",name.CString()); |
There was a problem hiding this comment.
Hm, seems to be a bit odd here, we're using the name to format a double?
source.AppendWithFormat("duk_push_number(ctx, (double) %s);\n", name.CString());
There was a problem hiding this comment.
Yeah, I copied that from how it was constructing the constants above. Those may both need to be fixed. I'll look at that.
…if/when TS intellisense honors it.
|
@shaddockh Hey, some neurons misfired on that string format, which uses the variable name, I have no idea what I was thinking :) I'll keep an eye out for any compiler warnings, though I don't think the cast to double was necessary, so in the end a few characters less code, and less code is always better 👍 Thanks! LANDING for Build 3! |
This PR adds new JS object literals to the Atomic namespace that allows for more proper enums instead of having to use constants. Now, in both TS and JS, instead of using:
you can use
The original constants are still present in the Atomic namespace so existing code doesn't break, but it is flagged to be removed. I also left the existing constants in the
d.ts, but am contemplating removing it. The repercussions would be that existing TS that uses the constants would have transpilation errors and would have to be converted to use the enums, but the JS would work fine. Also, it would clean up the auto-generated documentations and not have all those extra constants. Thoughts?Each of the existing legacy constants has a comment stating that it is deprecated and points to the corresponding enum.