Better DX for defining AbstractJavaScriptIndexCreationTask

kamranayub commented 5 years ago

This is more of a question/suggestion.

Looking at the tests for AbstractJavaScriptIndexCreationTask I'm wondering if we'll eventually be able to just express the index in raw JS since we can do Function.toString() to grab the raw definition to store. Could even provide some TypeScript static typing to help.

Example:

http://www.typescriptlang.org/play/index.html#code/C4TwDgpgBAkgdgEwgDwCIQGYEs5eFgezgB4AVAPigF4oAKBAgYwFcBbCOYALilIEpqlAgCMAVhEbAAUFMYAbAIYBnJVACCwpcABOCyQCkFANwUBlRtqxhg8JMgDC2iAvxFSygNZQA3lKj+oMEsTYGgAfVYFMFUaOAgAdyhTCGBiLUs4AHNyWj4ZAMDtAlDJCAQoSLAyHMYCOTkJVzgedJxMgBooJGxcJp5bFHQevEISCgFfAoLgAAssJQA6CKjFhQQEWgADStoAcgASb1r6xtGAX13Ow+6cEaIF4AJTHTbcs75NvgBuPwCz-ICmRSFRWuR8vymUCcwGY2jgUFm8yWlSUPwK-3+UhwoW0GD00AAqkoINpwQVmMS4Qp2C0Xlk0QEsAhaRlMj9MfJlKoiSSlGEAEIgHlU9hQFChRCqDTpPTAQwmcyWawDBxOFyjdxKLyTAK1ODpZiSAjaME6yFKZiQE15CFTRGLSrEYU5XbCpSXOjMARUSi0bxQCkkuDUiA8ZgLQMi6DvG2Q-x6pR1CALOQETK0e0LFF5dFSDlELRQHB2ahQOKJN0CoWU4PsXJAA

ayende commented 5 years ago

In general, I would absolutely love to have this feature, yes. Can you send a PR?

On Thu, Jul 11, 2019 at 5:50 AM Kamran Ayub notifications@github.com wrote:

This is more of a question/suggestion.

Looking at the tests for AbstractJavaScriptIndexCreationTask I'm wondering if we'll eventually be able to just express the index in raw JS since we can do Function.toString() to grab the raw definition to store. Could even provide some TypeScript static typing to help.

Example:

http://www.typescriptlang.org/play/index.html#code/C4TwDgpgBAkgdgEwgDwCIQGYEs5eFgezgB4AVAPigF4oAKBAgYwFcBbCOYALilIEpqlAgCMAVhEbAAUFMYAbAIYBnJVACCwpcABOCyQCkFANwUBlRtqxhg8JMgDC2iAvxFSygNZQA3lKj+oMEsTYGgAfVYFMFUaOAgAdyhTCGBiLUs4AHNyWj4ZAMDtAlDJCAQoSLAyHMYCOTkJVzgedJxMgBooJGxcJp5bFHQevEISCgFfAoLgAAssJQA6CKjFhQQEWgADStoAcgASb1r6xtGAX13Ow+6cEaIF4AJTHTbcs75NvgBuPwCz-ICmRSFRWuR8vymUCcwGY2jgUFm8yWlSUPwK-3+UhwoW0GD00AAqkoINpwQVmMS4Qp2C0Xlk0QEsAhaRlMj9MfJlKoiSSlGEAEIgHlU9hQFChRCqDTpPTAQwmcyWawDBxOFyjdxKLyTAK1ODpZiSAjaME6yFKZiQE15CFTRGLSrEYU5XbCpSXOjMARUSi0bxQCkkuDUiA8ZgLQMi6DvG2Q-x6pR1CALOQETK0e0LFF5dFSDlELRQHB2ahQOKJN0CoWU4PsXJAA

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ravendb/ravendb-nodejs-client/issues/208?email_source=notifications&email_token=AAA4RM2IPFO4UFKQCABXRHLP62NVXA5CNFSM4IANRUD2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4G6QGXPA, or mute the thread https://github.com/notifications/unsubscribe-auth/AAA4RMZHJ5DIZSUDJZDZZLTP62NVXANCNFSM4IANRUDQ .

-- https://ravendb.net/ Oren Eini CEO / Hibernating Rhinos LTD https://hibernatingrhinos.com/ Mobile: +972-52-548-6969 Sales: sales@ravendb.net Skype: ayenderahien Support: support@ravendb.net

yj7o5 commented 4 years ago

@ayende If this hasn't been taken care of, would love to send a PR. Thanks

ayende commented 4 years ago

Go ahead, would love it!

On Mon, Oct 12, 2020 at 7:00 AM Yawar Jamal notifications@github.com wrote:

@ayende https://github.com/ayende If this hasn't been taken care of, would love to send a PR. Thanks

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ravendb/ravendb-nodejs-client/issues/208#issuecomment-706844220, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA4RM6DF6RIRI3QG42YKQLSKJ5OFANCNFSM4IANRUDQ .

-- https://ravendb.net/ Oren Eini CEO / Hibernating Rhinos LTD https://hibernatingrhinos.com/ Mobile: 972-52-548-6969 Sales: sales@ravendb.net Skype: ayenderahien Support: support@ravendb.net https://www.facebook.com/pages/RavenDB/265907650186374 https://twitter.com/ravendb https://www.linkedin.com/company/hibernating-rhinos-ltd-/ https://ravendb.net/emailsignature/displayeventpage

yj7o5 commented 4 years ago

@ayende I haven't opened a PR yet since I have not been able to fully test again a running Raven instance but here's a quick POC I have: https://github.com/yj7o5/ravendb-nodejs-client/commit/9db45fe3e600edcea1ee4303614e7004d3d629de

Just wanted to make sure first you like the direction. So few things:

I removed the old test cases. Do we still want to keep those?
Any further refinements you'd like to see?

Will do a bit more clean up in the AbstractJavaScriptIndexTask class and make sure I can get the test suite fully running/tested and then create the PR. But so far the output definition matches with the old version.

ml054 commented 4 years ago

Hi, @yj7o5

The POC looks very good! The plan is to have strongly typed javascript indexes under current name: AbstractJavaScriptIndexCreationTask. We can do this in 5.0 (breaking change).

Also we want to expose current AbstractJavaScriptIndexCreationTask under new name (maybe: AbstractRawJavaScriptIndexCreationTask).

Currently I'm working on 5.0 client so once I have some consistent state I can send link to branch.

Another thing we need to check if it will work after you minify code (function names may change).

Please also remember to sign our CLA: https://ravendb.net/contributors/cla/sign

Feel free to continue your work on 4.2 branch. We can rebase your changes on top of 5.0 later.

ml054 commented 3 years ago

@yj7o5 I'd like to notify that issue: https://issues.hibernatingrhinos.com/issue/RDBC-421 is now in progress. I will be investigating your idea.

yj7o5 commented 3 years ago

Thanks @ml054 and just let me know if there are any changes required.

ml054 commented 3 years ago

@yj7o5 implemented as a part of https://github.com/ml054/ravendb-nodejs-client/tree/v5.0

ravendb / ravendb-nodejs-client

Better DX for defining AbstractJavaScriptIndexCreationTask #208