Khan / live-editor

A browser-based live coding environment.
Other
761 stars 184 forks source link

Date seems to be acting funny #612

Open EthanLuisMcDonough opened 7 years ago

EthanLuisMcDonough commented 7 years ago

Methods of instances of the Date object type are not working in the PJS environment. Look: https://www.khanacademy.org/computer-programming/date-object/5851520369033216

EricBalingit commented 7 years ago

@EthanLuisMcDonough

This is because of the way new <<NativeClass>> is handled by the live editor. The pjs sandbox only recognizes certain native class functions that can legally be called without new. Typed arrays are one example of native classes which throw an error when trying to construct/call them without new, and thus the sandbox does not handle them, giving an error <<TypedArray>> is not defined. When processing user code, the sandbox proceeds to transform all AST nodes which instantiate native objects ( and replaces those nodes with PJSOutput.applyInstance () - which constructs objects without using new. The result of Date () without new is a string, which is why it has no getMonth() method.

Invoking JavaScript Date as a function (i.e., without the new operator) will return a string representing the current date and time.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date

EricBalingit commented 7 years ago

@EthanLuisMcDonough

Also note that this is not the case in Web Projects - where NewExpressions are not transformed.

MatthiasPortzel commented 7 years ago

Thanks for that insight. Is there a way to work around this? Maybe adding a check to the live editor so it handles Date correctly?

EricBalingit commented 7 years ago

@MatthiasSaihttam

I do not have an answer to that, as far as altering to code of the live editor, and because of the policy and design decisions of KA, I doubt that proposals for using new would be accepted. Here is a link to the code that performs the application of NewExpression's by the live editor and here is a link to the actual code of PJSOutput.applyInstance().

The only way to get an actual date object from within the live editor ( from the user end ) would be by using a hack, however, processing provides day(), month(), year(), hour(), minute() and second().

See the examples here and here.

EricBalingit commented 7 years ago

@MatthiasSaihttam

Yes, that is because the live editor supports ( bears reference to ) those classes. But as I said, for example, if you tried to use any TypedArray, such as Int32Array, for example, it would throw "Int32Array is not defined".

EricBalingit commented 7 years ago

See my initial comment ( edited ) above.

MatthiasPortzel commented 7 years ago

Ok. There's one more relevant piece of code you haven't pointed out yet, here. This allows the Date object to be referenced inside the environment, like RegExp, without needing any workarounds to access the window object. This is in contrast to Int32Array or any TypedArray, which would work just fine in the live editor if they were "aliased" with the properties linked above. The problem is that most of the Functions which are linked there work perfectly fine without the new keyword, but Date doesn't. For example, new RegExp("a\\s*", "gi") works just the same as RegExp("a\\s*", "gi"). (The applyInstance code removes the new word and calls functions without it, as explained by the comments above that function.)

So we have a few options:

  1. Don't make any changes. Date.now() and other properties will still work, but new Date() won't.
  2. Handle Date the same way we would a user object, and not like other the Functions. I haven't tried this, and can't say for sure that it works, but there's so much code surrounding handling using functions, it might.
  3. Handle Date specially, with a check around here, funcName === "Date" a. The ES5 compatible way, which would look like this: return function () { return new (Function.prototype.bind.apply(Date, [null].concat(Array.from(arguments)))) } b. The ES6 way, return (function () { return new Date(...arguments)} )

All this to say, why the heck couldn't Date work without the new keyword‽

MatthiasPortzel commented 7 years ago

I just did some fairly quick testing. Option two will not work. It create's an object which appears to be an instance of "Date", but doesn't actually work. It's weird. I just checked out option three. It works just fine. However, it's obviously hack-y, so I'm not particularly inclined to create a PR before seeing if anyone else has a better idea. @kevinbarabash Do you have an opinion, seeing as you're the only KA employee that seems to ever handle this repository?

EricBalingit commented 7 years ago

The only way to get a new instance from the constructor is to use Function('return new Date()')(). Otherwise The Sandbox will transform NewExpressions. It's just one of those things that the sandbox will not provide as a feature without rewriting a good chunk of code.

MatthiasPortzel commented 7 years ago

@EricBalingit Could you clarify what you mean? I'm talking about adding an exception to the sandbox code to handle Date.

EricBalingit commented 7 years ago

@MatthiasSaihttam I see that now sorry for the confusion. :-)

EricBalingit commented 7 years ago

Would be very handy to also have typed arrays, but I understand that also involves more code and I'm not sure that that fits in with what the processingjs library's intent is.

kevinbarabash commented 7 years ago

I tried option 2 b/c that would've been my initial preference. It's interesting that the browser checks that the object we're calling Date methods on is actually a Date object. Option 3 works, but instead of special casing for Date, it seems like that approach could be use for all new for user created and built-in constructors alike.

MatthiasPortzel commented 7 years ago

You would think so. Personally, I don't really understand all the transforms the live editor does to the user code. Do you know what the initial point of the applyInstance code was? Because using option 3 for all objects is pretty much the same as not transforming the user code at all.