-
Notifications
You must be signed in to change notification settings - Fork 200
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TokenType structure and API #1041
Labels
Comments
By definition this is a breaking change, however if the creation of TokenTypes is always done by |
I think this structure may actually be missing one of the more important properties (tokenIdx/Idx) |
It may be a good idea to "hide" some of the internal properties using symbols instead of string properties. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The TokenType structure has many optional properties.
I believe this is due to legacy reasons as in the past TokenTypes could be classes with static properties. The fact this structure only has a single mandatory property (name) means that any API that accepts TokenType (e.g CONSUME) could also accept (by mistake) other similar structures. For example a JS Function has name property and is therefore acceptable anywhere a TokenType is accepted.
I believe the TokenType structure should more closely match what is returned by "createToken"
which means a-lot less if any optional properties.
Also getting rid of the upper case property names would also be advisable.
The text was updated successfully, but these errors were encountered: