search

mozilla / positron

a experimental, Electron-compatible runtime on top of Gecko
Other
562 stars 64 forks source link

Move electron to a submodule. #59

Closed brendandahl closed 7 years ago

brendandahl commented 8 years ago

Modules is at v 1.1.3 See fixes in top two commits: https://github.com/mozilla/positron-electron/commits/master

For now I have electron as the upstream for positron-electron, but we can change that to tofino-electron once I get confirmation that it's being used.

mykmelez commented 8 years ago

Nice. This'll complicate certain other changes that require modifications to Electron (like my fix for #20), but it's still useful to be able to track those changes, and the version of Electron we're using, more explicitly.

We should do the same for Node, ideally using the version of Node that Electron itself depends on via a submodule (at least until we replace it with SpiderNode), so we know for sure that we're using a compatible version of Node. But we can do that separately.

The only issue I see is that the README is now out-of-date. It should get updated in conjunction with this change to explain that you need to initialize and update the submodule after cloning (or in conjunction with cloning) before you can build the project.

I also wonder if it makes more sense to put the moz.build instructions into this repo rather than that one. I think that's possible, because complete source and destination paths for EXTRA_JS_MODULES can be specified at any level of the tree. And it would be one less change to Electron.

mykmelez commented 8 years ago

@brendandahl I saw an issue and have submitted a PR in https://github.com/brendandahl/positron/pull/1 to resolve it.

Now I'm seeing:

JavaScript error: file:///Users/myk/Projects/positron/positron/test/hello-world/index.html, line 9: NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS: [JavaScript Error: "v8Util.createIDWeakMap is not a function" {file: "resource:///modules/ModuleLoader.jsm -> resource:///modules/renderer/api/remote.js" line: 10}]'[JavaScript Error: "v8Util.createIDWeakMap is not a function" {file: "resource:///modules/ModuleLoader.jsm -> resource:///modules/renderer/api/remote.js" line: 10}]' when calling method: [nsIDOMGlobalPropertyInitializer::init]

I saw that you implemented createDoubleIDWeakMap using WeakMap. You might be able to do that for createIDWeakMap too, but the version of Electron that we're currently using has a different API, an IDWeakMap constructor, which I had to implement differently in positron/modules/atom_common_id_weak_map.js in order to get stuff working, since WeakMap keys can't be primitives, and the version of Electron we're currently using uses integer primitives as keys (which is presumably also true for the new version, at least for createIDWeakMap, although createDoubleIDWeakMap looks like it uses objects as keys).