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

Relying on eval? #12

Open
tmillr opened this issue Aug 18, 2021 · 11 comments
Open

Relying on eval? #12

tmillr opened this issue Aug 18, 2021 · 11 comments

Comments

@tmillr
Copy link

tmillr commented Aug 18, 2021

return $Function('"use strict"; return (' + expressionSyntax + ').constructor;')();

Hi, I was just wondering why eval is being used here instead of just hard-coding the values? Is this done in order to avoid, say, throwing uncatchable parser/syntax errors that would otherwise occur in older ECMA environments? If that is the reason, I think it would at least be useful to maybe have another version of this package (whether internal, or external via a separate package) that would always be able to query all intrinsics in any modern environment (say esnext) regardless of whether eval is allowed or not.

I think it would also be nice if there were a way to access all of these intrinsics at once, say in a collection or array, or even a plain object/hash whose keys could be iterated over.

Instead of doing it all from scratch, I've been trying to find a package that would be able to take in any unknown value where:

((typeof value === "object" || typeof value === "function") && value !== null) 

and then test it in order to see if it is a is a builtin/intrinsic object (e.g. Math), or builtin/intrinsic Function (e.g. Date). It is this issue that led me here to this package. I suppose most or many of these values simply exist on the global object (in which case one could perhaps simply test against every value in globalThis), but that doesn't seem to be the case for all of them...

Finally, I was just wondering what the benefit is of querying these builtins/intrinsics by their language spec name/syntax? Is it just done that way to have a one-to-one mapping in regards to how they are listed in the spec? I'm honestly just curious to know if there's something additional that I might be missing here, that's all.

Sorry for being a bit of a noob, and thank you beforehand for hearing out my concerns and any information you can provide regarding these questions!

@ljharb
Copy link
Owner

ljharb commented Aug 18, 2021

Yes, it's impossible to hardcode the values, because the only non-eval way to get to them is syntax that would break the entire program in older browsers.

What use case do you have for iterating over all intrinsics? Why does it matter if a value is an intrinsic or not, if you don't know which one it is?

It's indeed done that way to have a one-to-one mapping; the purpose of this package is to be used in polyfills/shims which also match the spec.

@tmillr
Copy link
Author

tmillr commented Aug 18, 2021

What use case do you have for iterating over all intrinsics? Why does it matter if a value is an intrinsic or not, if you don't know which one it is?

I'm working on an experimental package (that would be used in development I suppose) that takes an unknown value at runtime and recursively expands and prints out its typescript type/shape, especially for any plain objects or plain arrays it finds while walking them. I think this could be useful in typing random objects say JSON responses from a web server instead of doing the entire thing by hand (perhaps it would cut the time in half, although some manual editing is still gonna be involved). I want to to be somewhat compatible with values such as intrinsics if one is found or come across for any reason (so that way my module won't try to walk Math for instance and will instead simply emit Math for its type). Perhaps there's already some code in the typescript repo that already does something similar, but part of this package is simply learning experience for me as well (maybe it will be my first npm pkg).

Here is what I've currently got that determines if an object is a plain/default object that should be expanded/walked or not:

function isRegularObject(value: unknown) {
    return (
        typeof value === "object" &&
        value !== null && (
        Object.getPrototypeOf(value) === null ||
        value.constructor === {}.constructor
        )
    );
}

and I've got this for plain/default arrays:

function isDefaultArray(value: unknown) {
    return typeof value === "object" &&
        value !== null && Object.getPrototypeOf(value) === Object.getPrototypeOf([]);
}

but the former fn returns true for Math etc. and so will start walking Math which I don't want.

It's indeed done that way to have a one-to-one mapping; the purpose of this package is to be used in polyfills/shims which also match the spec.

Ah I see, thank you for that!

@tmillr
Copy link
Author

tmillr commented Aug 18, 2021

What use case do you have for iterating over all intrinsics? Why does it matter if a value is an intrinsic or not, if you don't know which one it is?

Oh boy I'm sorry, I'm not sure I even answered your question! That's a good question.

Well, if the unknown value matches an intrinsic, then I was figuring that maybe a type could be emitted by doing something like:

`${value.name}Constructor`

or

`typeof ${value.name}`

for functions, and then

`${value.constructor.name}`

for objects, although it is something I am still investigating and I am not sure that is going to work in every case. I also just realized my isRegularObject() fn would think that prototype-less objects are regular objects (and from there type HasNullProto = { key: string } would actually incorrectly inherit Object.prototype keys I believe), which wouldn't be entirely correct.

@ljharb
Copy link
Owner

ljharb commented Aug 18, 2021

a) you can never rely on .constructor being present or correct
b) most intrinsics aren't constructors at all

Runtime comparison to intrinsics would happen after type information has already been deleted - at type-time, you can't rely on runtime values.

@tmillr
Copy link
Author

tmillr commented Aug 18, 2021

I understand. It seems like alot of different algorithms in the spec itself do rely on "constructor" being correct however via Get(obj, "constructor"). I'm having a hard time though finding where in the spec it actually details how and when this property is set initially, and I only just now found out that it comes from and exists on an object's prototype.

But basically this module I'm mucking about with is only intended to be used for assistance with manually creating type definition files at design time. It is not intended to be an end all solution either. For example, say you want to emit a type for a parsed JSON server response object of unknown type/shape that comes in response to sending a request to an API endpoint of someone else's website/server. This object typically has the same set of keys (repeated requests to the same url return similar or same JSON structure/type) whose values are typically of a certain type. Well one way might be to just JSON.stringify() it then edit the values manually by hand (true becomes boolean, "hello" becomes string, unquote the keys, add an index signature + mark all properties as optional just in case the website evolves faster than our type definitions, etc.) but I'd like a module to do that part for me among other things. I may have widened the scope of what I am trying to accomplish a bit too much in trying to tackle the typing of non-JSON objects/values at runtime however.

@ljharb
Copy link
Owner

ljharb commented Aug 18, 2021

At design time you don't have any runtime values tho, so i'm not sure how you could use what you're asking about.

It's a very very hard problem to try to infer and then emit a type for a runtime value, at runtime, and i don't think this library would help much.

@tmillr
Copy link
Author

tmillr commented Aug 19, 2021

I'm sorry I'm probably not explaining it too well. This is what my inspiration was: There's a simple npm pkg here for interacting with the Robinhood API. It has a .d.ts file which defines itself and its methods. The methods simply send a request to Robinhood's servers and then return the response body as a parsed JSON object. I wanted to add types (to their .d.ts) for them for their methods so that anyone who depends on their module can have a fairly good idea what the methods are returning instead of just any (the methods are actually just using the request npm pkg which itself uses callbacks to "return" the server response). Now, I thought something like the module I'm working on might make things slightly easier and faster in accomplishing generating/authoring those types or shapes of the server responses. The manual way to do this, I gather, is to just enter the REPL and console.log the JSON server responses received from calling each relevant method of robinhood npm pkg, then copy/paste/edit that into their .d.ts file. The edit part simply means converting that text to a valid ts type/shape by hand.

My module would enable the same workflow, but more or less automate the edit part if it was required() and used in the REPL as well during this process. It would accept a JSON value (or what I really wanted was any js value lol, so that my module would become more universal in its potential use-cases) and output a string that is the ts type definition that can be copy & pasted into a .d.ts file from there. But I agree that it wouldn't be adding a ton of value and that the whole process is still a manual one. So basically I started working on this module of mine and then thought to myself, how do I make this compatible with other potential situations people might encounter when trying to author ts type definitions (that aren't simply limited to JSON values)? But really all of these use-cases might be quite rare perhaps, and .d.ts files should only ever be authored this way when dealing with unknown types that need to be more or less "acquired" at runtime (as is the case being an outsider while trying to work/interact with Robinhood's own, "private" API).

Thank you for the advice and knowledge you've shared with me here however, as well as answering my questions about this repo. I really appreciate it!

Edit: fwiw here's an example of one of the types I did manually by hand

@ljharb
Copy link
Owner

ljharb commented Aug 19, 2021

I think it has to always be manual - because you can't programmatically know what possible types something will have. You might make 100 requests and get an integer, and make a 101st and get a string. You can only document the types of a thing if you authored it, or if you inspect the code/docs and match them.

@tmillr
Copy link
Author

tmillr commented Aug 19, 2021

I think it has to always be manual - because you can't programmatically know what possible types something will have. You might make 100 requests and get an integer, and make a 101st and get a string. You can only document the types of a thing if you authored it, or if you inspect the code/docs and match them.

I think I get it. So in essence it is more important for a type definition to be guaranteed correct than to exist at all (or exist but only exist as any)?

For the sake of argument, lets assume that in my example here the thing we're trying to type is guaranteed to be a plain {...} JSON (or JSON-derived) object (not a JSON string, array, etc.), and there's truly no other way to author type definitions for this object. One thing that then make things kinda interesting is the ability to mark properties as optional while including an index signature... in that case such a type becomes almost like a hint, and in such circumstances the question then becomes: is such a type, or its existence, truly semantically incorrect? Or, perhaps it is correct but simply a bad practice? Maybe there's already a golden rule or best practice for this case, trying to author such a type, that I am not aware of. You know what would be cool is a repo dedicated to best practices for js and ts (for noobs and semi-noobs like myself) if there isn't already, with explanations for why each example is a best practice. I know we already have tools like ESLint and friends, but some best practices go beyond mere code (such as in this case) (and I apologize if such a repo already exists, or if I'm stating the blatantly obvious here).

At the end of the day, I understand that this particular pkg robinhood isn't like most, cannot replied upon, and is merely provided for convenience, and so maybe then making the module that I was going to make is a waste of time as it is for a use-case far too rare. Plus, I can see how it could be bad to encourage authoring types in such a manner, and perhaps merely publishing such a pkg could encourage such a thing. I don't know.

@ljharb
Copy link
Owner

ljharb commented Aug 19, 2021

Yes, an incorrect type definition is far far worse than none at all.

@archmoj

This comment was marked as spam.

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

3 participants