Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

String values of enums #264

Open
bendemeyer opened this issue Dec 28, 2023 · 6 comments
Open

String values of enums #264

bendemeyer opened this issue Dec 28, 2023 · 6 comments

Comments

@bendemeyer
Copy link

Is there a reason that the enums defined in this package don't have their actual string values assigned to them? I understand that it would be a bit weird from a philosophical standpoint, but I'm wondering if there's a practical reason to avoid it as well. I think there would be some practical benefits to providing those values, so if there are no practical drawbacks I think it's worth considering.

For example, I might want to pass a value from record.Type to a function that takes a string as an argument. Because the record.Type enum doesn't have values assigned in this package, I have to do that with as unknown as string, which is an inconvenience.

A less common but more impactful example would be if I wanted to type something such that it allowed string literals that are part of an enum. If the enum in question has values, this is doable with const thing: `${MyEnum}`;, but this can't be done with the enums as they're currently defined in this package.

If this would be desirable, I can submit a PR for it.

@bendemeyer
Copy link
Author

Looking for other examples of this behavior, I noticed that there actually are some enums in this project that have values assigned. Assuming that's acceptable then, I'll try to put together a PR to add some of the missing ones.

@MrRob
Copy link
Collaborator

MrRob commented Jan 2, 2024

@bendemeyer Yea to be honest I'm not sure why they were set up the way they were. Not sure if @johnogle222 might know/remember? In my own code when I run into this I'll usually just cast record.type to String, like: String(record.type) instead of doing "as unknown as string". Slightly less annoying maybe?

@ShawnTalbert
Copy link
Contributor

ShawnTalbert commented Jan 2, 2024 via email

@johnogle222
Copy link
Contributor

Staying true to the abstraction and not making assumptions about what might change underneath was one big part of the motivation.

Not wanting to go through and discover all of the various actual string literals was another motivator. It's been, like, a decade now? But I'm pretty sure there were examples of situations where the enum value didn't exactly match the string[0], and I didn't want to encode absolutely false values into a typing library that might cause bad assumptions or even introduce bugs in code itself. The actual values could certainly be discovered if we wanted either through brute force or a script. And realistically NetSuite will likely not change any of these values without versioning the API. But sticking with the abstraction as it was documented felt like the "most right" thing to do when putting this together.

[0] I can only provide a hypothetical as it's been too long. But say there was a record.Type.CHECK enum documented and available. You couldn't just assume that the underlying string was "CHECK" or "check". It might be something like "NS_CHECK".

@MrRob
Copy link
Collaborator

MrRob commented Jan 3, 2024

Thank you for the wisdom @johnogle222 and @ShawnTalbert :)

@bendemeyer
Copy link
Author

Staying true to the abstraction and not making assumptions about what might change underneath was one big part of the motivation.

This makes a ton of sense to me, broadly speaking, but I think the way this project is fundamentally set up undercuts it a bit. We don't have defined interfaces for each SuiteScript module, instead each one is represented by a file and the exports on that file represent the type of the module itself.

I'm guessing this was done so that we could map all the N/* modules to this project via tsconfig and have everything "just work," but it also creates complications like this where the module files need to export actual enums, as opposed to defining an interface that contains the appropriate enum types (similar to how the UserEventType enums are handled, since those don't have to be directly exported from a module).

I'm going to think on this a bit more. I can see a cleaner version of this where interface definitions are used for everything, and explicit module declarations handle the N/* mappings, but I'm not sure how to set that up in a way that's effective from a practical usage standpoint.

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

No branches or pull requests

4 participants