-
Notifications
You must be signed in to change notification settings - Fork 125
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
Is this still being actively maintained? #38
Comments
So yes, I'm still maintaining the project, and will happily review any pull requests. I'm really glad that you have been working on getting some of these issues fixed, as they've all been on my radar for a long time now, it's just been hard to find the time. Let's get some of these patches landed! |
Great! We'll clean it up and get it over to you. |
Hi Nate, I've finished my own unit tests and have fixed the items below. Can you open a new branch 0.1.0 that I can push up to? There's some issues below that we'd need to address before any sort of merge into a main/master. Here's the branch: https://github.com/trevorlinton/NodObjC/tree/0.1.0 Changes:
|
Hey @trevorlinton Nice looking branch, and list! So I've added you as a committer to this main repo. You can push to a new branch if you want and we can take a closer look and get things merged to the I really appreciate you diving in and doing what's been on my TODO list for quite a while! Cheers! |
@TooTallNate Hi Nate, after talking with some Apple engineers the general consensus was that ID's are a nomenclature and that Class and ID effectively mean the same thing since Classes are just Objects in obj-c (but only have significant differences in compiler validations and when integrating with C/C++ bridges). This may actually be good news but brings up a design ramification I wanted to clear through you before I do anything. Should the ID and Class be merged together? If so, should the following assertions be true: var someInstance = $.NSString('alloc');
assert(someInstance !== $.NSString);
assert(someInstance instanceof $.NSString);
assert(someInstance instanceof $.NSObject); // maintain heirarchy in obj-c
assert(someInstance.class() === $.NSString);
assert(someInstance.class().super() === $.NSObject);
assert(someInstance.super() == someInstance); // ???????
// what happens here? and instance has a super of what? The NSObject "instance" i suppose?
someInstance.toString(); // Format: '[ '+ClassName+' '+pointerAddress+' ]' [NSString 0x00000003 *]
// note that the * just tries to differentiate between a instance (id) and a class.
someInstance.class().toString(); // [NSString 0x00000001]
someInstance.class().super().toString(); // [NSObject 0x00000000]
someInstance.super().toString(); // [NSObject 0x00000000 *] ????
// what happens here? once again do we infer an "instance" of NSObject as the super? or?
assert(someInstance.isInstance() == true)
assert(someInstance.isClass() == false)
assert($.NSObject.isClass() == true)
assert($.NSObject.class() === Object.prototype) // ??? In addition static or '+' methods are only accessible by Class "definitions" and '-' or instance methods are only available by ID "definitions" (if you will, I say definitions as both Class/ID would come out of the same object in node). Finally, extending a class simply extends its '+' methods and applies a new inheritance chain? In addition should extending a class into a new one immediately register it within the NodObjc main object for consistency? |
I mean I wouldn't call them the same thing... Class instances are a type of Object, but vice-versa isn't true. Just look at the Obj-C runtime reference. You can see that there's functions (prefixed
Well NodObjC is a bridge right? 😉
So... all of those assertions seem fine to me in theory, however I'm not sure if we'll be able to get assert(someInstance.super() == someInstance); // ???????
assert(someInstance.super().toString(); // [NSObject 0x00000000 *] ????
// what happens here? once again do we infer an "instance" of NSObject as the super? or? Both of those bases seem kind of weird to me. A assert($.NSObject.class() === Object.prototype) // ??? I would think
Well if we could get
It already does that in the Let me know if that helps! |
@TooTallNate All of your points were fairly spot on. I tried to do some more refactoring but realized I was walking down a rabbit hole and have since stopped. I have found one issue which is perplexing, using core.objc_getAssociatedObject and core.objc_setAssociatedObject to hold onto a pointer within ID.wrap seems to sometimes cause assertion violations (as the incoming/return pointers are not equal), I believe this is due to the objects original memory pointer being recycled by obj-c's runtime. In addition, it seems it prevents v8 from gobbling up anything passed through ID.wrap, which is obviously an issue. I was wondering if this was done for speed or if it served another purpose? I've also added a test-case memtest.js that tries to cause memory leaks. So far (there's still a ways to go) but it seems most of the memory issues are resolved. You can see the updates here: trevorlinton@0184e91 Thoughts? |
IIRC, the NodObjC/test/array-references.js Line 13 in 938b346
As for your patch, I'll try reviewing that a bit later today. Cheers! |
@TooTallNate Ahh, that makes more sense, I was shoring up the code to pass all the unit tests and the array-references was one of the outliers I was struggling with. That helps, thanks. I'll soon have a finished patch with all the unit tests passing. And the latest mem-tests i've ran allocate 2GB of structs, to delegates and function pointers without a single bit of memory leaks! (well, atleast on the javascript/v8 side of things). |
@TooTallNate All unit tests are passing, memory references seem clear and i've successfully decoupled Class from ID. Note there were minor changes to some unit tests, most were simply changing the "require" file/module; all of the code of the unit tests were kept. |
@TooTallNate I pushed up branch 0.1.0, docs need to be revised, maybe some more testing and your thumbs up and we can set it in motion! |
@trevorlinton Hey Trev, so I'm checking out your branch now. First issue is that the dependencies in the |
Let's continue this in #39. |
Hi,
I've been working heavily in nodobjc for quite a few months and have fixed a vast amount of bugs i've found, including:
I'd like to push these up but the API has changed (very slightly), the dependencies have changed, and I need it (quickly) for a project i'm doing.
I'm considering creating a derived project from nodobjc for the changes i've made so I can get into NPM in the next two-three weeks but I wanted to first check here to see if this is being actively maintained, if so i'll push the changes up here. However if you think this is something you'll not be maintianing actively going forward my company can commit to maintaining its feature set.
I guess i'm asking for your recommendation on what we should do concerning nodobjc. (BTW, any new project we'd be happy to add a disclaimer that its derived work from nodobjc and give you credit, and if the changes are integrated into nodobjc we'd redirect back, or, if you can add us as a contributor/collaborator we can continue to maintain nodobjc through your repo).
Thanks!
Trevor
The text was updated successfully, but these errors were encountered: