Finish implementing APIs for Chrome #63
Conversation
|
|
||
| // TODO: DATA | ||
| rpc.registerTypeConverter(types.DATE, (_, info) => new Date(info.value)); | ||
| rpc.registerTypeConverter(types.LIST, lists.create); | ||
| rpc.registerTypeConverter(types.OBJECT, objects.create); | ||
| rpc.registerTypeConverter('ObjectTypesNOTIFICATION', notifications.create); |
There was a problem hiding this comment.
We should make a static string for ObjectTypes
|
Looks good. |
| exports.create = create; | ||
|
|
||
| function create(realmId, info) { | ||
| let notification = new Notification(); |
There was a problem hiding this comment.
Is this a class for the sake of consistency, or is there a type check somewhere? Otherwise, couldn't it just be
return { realmKey: realmId, idKey: info.id };
There was a problem hiding this comment.
It's a class because technically it should have a constructor named Notification, which currently does nothing, but will have methods added in the near future. 😄
That was easy!
Since notifications are called synchronously after a write, we fake it by calling them manually for now. The future plan will be more involved, so some of that is stubbed out.
Which only consists of sortByProperty
appden
force-pushed
the
sk-chrome-apis
branch
from
e207f9c to
4c0cc57
Compare
| @@ -1,63 +1,40 @@ | |||
| 'use strict'; | |||
|
|
|||
| let keys = require('./keys'); | |||
There was a problem hiding this comment.
Its really annoying to have files for every reused object. make these a property on an existing object if possible
alazier
changed the title
Assigning to non-existent index will throw an exception and all tests pass in Chrome!
| @@ -55,9 +60,24 @@ - (instancetype)init { | |||
| if (self) { | |||
| _context = JSGlobalContextCreate(NULL); | |||
|
|
|||
| // JavaScriptCore crashes when trying to walk up the native stack to print the stacktrace. | |||
| // FIXME: Avoid having to do this! | |||
| static void (*setIncludesNativeCallStack)(JSGlobalContextRef, bool) = (void (*)(JSGlobalContextRef, bool))dlsym(RTLD_DEFAULT, "JSGlobalContextSetIncludesNativeCallStackWhenReportingExceptions"); | |||
There was a problem hiding this comment.
Don't think we should be using private methods here. What happens if we don't do this?
There was a problem hiding this comment.
It crashes hard whenever any of our methods or property getter/setters throw an exception. Probably due to JavaScriptCore not supporting C++ lambdas. I agree with your sentiment, but that's why I left a FIXME. I think if we switch to regular functions from lambdas we won't need this, and I thought we'd probably do that when refactoring this file away from Objective-C.
|
Lets make issues for any remaining items and get this merged. |
appden
changed the title
Finish implementing APIs for Chrome
Optional property bug fixe/case insensitive queries
No description provided.