Skip to content
This repository was archived by the owner on Jan 8, 2020. It is now read-only.

Added Date support for RhinoWrapper#10

Open
igelbox wants to merge 2 commits intofreemarker:2.3-gaefrom
igelbox:rhino-dates-ci
Open

Added Date support for RhinoWrapper#10
igelbox wants to merge 2 commits intofreemarker:2.3-gaefrom
igelbox:rhino-dates-ci

Conversation

@igelbox
Copy link

@igelbox igelbox commented Nov 22, 2013

use class-injection in 'org.mozilla.javascript' (unfortunately NativeDate class is package protected)

alternative is reflection-based implementation in my rhino-dates branch

(use class-injection in 'org.mozilla.javascript')
@igelbox
Copy link
Author

igelbox commented Nov 25, 2013

org.mozilla.javascript.NativeDate? Yes, it was always there (since Publish Rhino as open source). But getJSTimeValue-method available since this commit.
Dynamic checks may allow to publish stable version of FreeMarker that will fail at runtime. I think it would be better to use unit tests.

@ddekany
Copy link
Contributor

ddekany commented Nov 25, 2013

So, the method it's there for, like, 10 years. Well, in the 2.3.X line we still have to keep backward compatibility even with that... I know, it's extreme. As of unit tests and so, I don't get what you mean. So there's an app that uses some early Rhino, and they update FM under it. They had no proper date support earlier, and they won't have it now either. Things keep working. But if now they get a linkage exception... that's bad. Speaking of which, even if the app has a fresher Rhino, that we suddenly treat dates properly can break the app too. So this will have to be enabled only if they explicitly ask for it. Ouch. OTOH that means that then we can demand a high enough Rhino version, instead of doing trickier checks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants