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.

Sign up for GitHub

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

Jump to bottom

Define project structure #1

Closed
Viatorus opened this issue Jan 21, 2017 · 25 comments
Closed

Define project structure #1

Viatorus opened this issue Jan 21, 2017 · 25 comments

Comments

@Viatorus
Copy link
Member

Viatorus commented Jan 21, 2017 •
edited

Hey guys,

I just started defining the project structure of LokiJS2. Its the first time I am doing this, so maybe someone can help me out.

What I used so far:

  • jsdoc (documentation)
  • eslint (coding/formatting style, not configurated yet)
  • webpack and babel
    • bundle src-files to one file (src/** -> lib/)
    • solve ES6 imports with babel
  • karma + jasmin (test platform + unit test framework)
    • uses karma-webpack to enable import .. form ...
  • uglify-js with specific branch to harmony -> supports ES6
  • istanbul
  • benchmark

What is missing:

  • .travis.yml, bower, npm
  • phantomjs (currently does not support ES6).

For me open questions:

  • Do we want to code ES6 and transpile to ES5 (with babel)? For me: Just ES6.

Please feel free to give helpful hints/commits for the initial project structure. ;)

Started with: techfort/LokiJS/issues/525
@techfort
Copy link

hey @Viatorus what you have there is good. I'm thinking that (to answer your last question) the project will be entirely es6 transpiled for browser environments, with only partial transpilation for node (e.g. i work a lot on lambdas which run on node 4.3.2 so transpilation is only partial, eg. import syntax if you use commonjs, some features like destructuring which are not supported in 4.3.2).

As far as files structure i don't have strong preferences but obviously each component would have its own file.
My biggest doubt is that LokiJS 1.x is heavily OOP-oriented, while i would like 2.x to be FP as much as possible, so really you shouldn't have a (let's say) uniqueindex.js with a UniqueIndex class in it, but rather a bunch of functions that cover that aspect of the lib... I will leave this up to discussion, I don't feel like imposing programming paradigms.

@Viatorus
Copy link
Member Author

I tried to add istanbul.js (code coverage) to the project. It does work if the coverage is 100%, else it returns an interal error that I currently cannot handle...
It seems to me, that some dependencies have still difficulties with ES6...

Well, for me OOP seems cleaner than FP and also prefer to put different functionalities in different files but I am also open for everything else. Just find a solid and uniform way. ;)

@Viatorus
Copy link
Member Author

Okay, I got istanbul working with commit 44b0a1c. :)

@Viatorus
Copy link
Member Author

Viatorus commented Jan 23, 2017 •
edited

With webpack we have the possibility to define multiple entry points to split the library into independent build targets ( from here). So the user can select/load options what he really needs (like plugins).

We could split lokijs into core, adapters and inverted index (e.g.).

Now to the hurdle: Splitting the source code into files. Currently the source code dependencie looks like this:
loki-uml (from Object Playground).

Its not to easy to understand the dependencies. o.O

  • Would it be enough to outsource Collection and Adapter?
  • KeyValueStore could maybe be implemented using ES6 Maps (but I don´t find a place where it is used inside loki ???).
  • We should definitely use ES6 classes where we use ES5 function classes.

@Viatorus
Copy link
Member Author

I updated the structure. Now benchmarking is possible.

Still investigate on how to create optional dependencies in webpack....
LokiJS-core could be used with/without adapters/inverted-index.

@Viatorus
Copy link
Member Author

Now unit tests in node are possible. But I couldn't get coverage to work inside node...

@Viatorus
Copy link
Member Author

Viatorus commented Jan 26, 2017 •
edited

I still investigate in analyzing the code structure. I created an UML for that.

uml

The dependencies of each class is resolved (to other classes and functions). The functions dependencies are not resolved. The other adapters (crypto, angular, jquery-sync) have only external dependencies. Edit: It is not completely without missing dependencies as I see know.

@techfort Could you explain to me what ExactIndex really is or should be? I don't see any useful code usage. Inside the source code it looks like it is never really used besides of collection creation.

@obeliskos
Copy link
Collaborator

hey @Viatorus , I just made a commit in which I added a unit test for and noticed that it was using lokicore instead of src. I tried doing an npm run build but errors abounded.

Are we in a state where we can manually run unit tests on lokijs2 and what would the process be? I went ahead and checked in since I already tested on lokijs project but would be good if I could test locally at least.

Let me know if someone else set that up, thanks!

@Viatorus
Copy link
Member Author

Viatorus commented Mar 5, 2017

Hey @obeliskos,
thank you for your contributions. I added your changes to the "new" lokijs 2 module system in src/core.
I adjusted the unit tests to use ES6 modules from src/core.
Unit tests can be run manually with: npm run test or test:web or test:node.

npm run build also works. These errors are mostly from the old lokijs.js and can be ignored. I just wanted to leave it in this developer branch to better keep track of changes between v1 and v2.

@obeliskos
Copy link
Collaborator

Ah thanks, I think I get how this is set up now... nice!

thanks for moving those changes.

@obeliskos
Copy link
Collaborator

A couple of notes, I removed eslint erroring on crlf as that is what I often use. If there is a way to have it convert that to common lf that's would be fine to add back.

I also renamed the two specs in /spec root so they do not cause errors when running unit tests

I did an npm run build and it did rebuild the minified version and map in lib but not the unminified version. Shouldn't it rebuild the unminified version too?

@Viatorus
Copy link
Member Author

With npm run lint:fix all files will be formatted like defined in eslint.
I am working on a linux machine so it doesn't matter for me which line ending we use.

I removed the 2 specs because this was the rest of my template project.

There is currently a problem with UglifyJS (it does not support ES6). If you run npm run build the minified files are not minified. Run npm run dev to get a real unminified version.

@Viatorus
Copy link
Member Author

Viatorus commented May 6, 2017

@obeliskos @techfort
I would like to enable travis for this repository.
However, under github.com / Authorized application / Organization access I got:

LokiJS-Forge - Access request pending

How can I solve this?

@Viatorus
Copy link
Member Author

Viatorus commented May 22, 2017 •
edited

Hey, this week I want to start with porting LokiJS to LokiJS 2 and refactor the source code.

  • Use latest commit in techfort/LokiJS/tree/v2.0.0
  • Split ES5 "classes" into ES6 classes, each in separate files (again)
  • Update unit tests
  • Refactor inline functions with ES6 arrow functions (e.g. removing self = this;)
  • Add a leading _ (dash) to member variables/private functions of a class, use getter/setter syntax for variable access
  • Update var to const or let.

I think, these changes should not alter the behavior of LokiJS.
Did I missed some lightweight refactoring possibility?

@obeliskos
Copy link
Collaborator

obeliskos commented May 22, 2017 •
edited

Hey Viatorus, sorry I was tweaking one last 'stability bug fix' since it was so small a change... but I just now made that final lokijs.js commit to complete stability fixes refactoring.

I only foresee sandbox webpage, examples, and wiki updates from now on to document these changes.

@Viatorus
Copy link
Member Author

No problem. :)

@Viatorus
Copy link
Member Author

So I have done some refactor things I mention here #1 (comment).

My current problem is the renaming of variables, which should be used only internal and not by the user. Using getter/setter symantic is extremly slow compared to single value assign...

@techfort
Please allow different services (travis etc.) for LokiJS-Forge or give obeliskos/me more rights to handle it.

@obeliskos
Copy link
Collaborator

Sounds great :)

Are you able to run the old benchmark to guage effects of any single refactoring? If they are not used in critical paths it might not effect performance too much...

It might also be good a time for gauging any changes to performance that the new node 8.0.0 version might offer or carry....

@techfort
Copy link

@Viatorus @obeliskos great to see you guys hard at work with this. How do i give you the right to enable services? i made you repository admins so i thought you would be able to do it. Alternatively let me know what services you need and i'll enable them.

@Viatorus
Copy link
Member Author

@obeliskos
I didn't benchmark LokiJS 1/2 yet, I just had a look at stackoverflow.com and JSPerf. The node v8 hint is a good idea.

@techfort
Please check out the pending permissions for LokiJS-Forge.
image

@techfort
Copy link

techfort commented May 31, 2017 via email

@Viatorus
Copy link
Member Author

Viatorus commented Jun 1, 2017

Thank you!

@obeliskos
Why does LokiJS use its on "benchmark tool" instead of benchmark.js?

@Viatorus
Copy link
Member Author

Viatorus commented Jun 1, 2017 •
edited

I would like to split LokiJS2 into single packages (but still located in this repository) using npm scoping.

So my first idea is ((at) --> @):

(at)lokijs/core
(at)lokijs/full-text-search
(at)lokijs/adapter-local-storage
(at)lokijs/adapter-indexed-db
(at)lokijs/adapter-nativescript
(at)lokijs/adapter-crypted-file
(at)lokijs/adapter-incremental
(at)lokijs/adapter-memory
(at)lokijs/adapter-partition

Please have a look at https://github.com/angular/angular and https://www.npmjs.com/~angular.

What do you guys think?

@obeliskos
Copy link
Collaborator

Interesting... i wasn't aware of that option. @techfort has done all the npm publishing so i will let him weigh in on that but that sounds like it could have been useful for some of our persistence adapters for 1.4 branch.

As for benchmarks, the logistics for timing aren't that difficult and no added (dev) dependencies was nice and i believe the timer resolution was more than sufficient, but if you would like to re-engineer some benchmarks for 2.0 and you think it would help feel free to use best in class libraries if they are accurate enough (repeatable with minor variance between runs).

@obeliskos
Copy link
Collaborator

Looks like you are laying major groundwork to build this new foundation 🚀

Maybe once things settle down I can start figuring out what's going on... I can see I have a lot of new things to learn.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@Viatorus @techfort @obeliskos