-
Notifications
You must be signed in to change notification settings - Fork 31
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
CommonJS require() loader #150
Comments
Erm, the "main module" I described in that issue is actually intended to be the entry point for using qbrt as a library within another Node package, i.e. the value of the package.json's main field. Right now qbrt only provides an executable, and it only makes sense to install it globally; but I could imagine its functionality being useful to another Node package, if its APIs were exposed via a main module.
Yes, I'm happy to discuss this! Note that ES6 modules are steadily being implemented in Firefox, and support for module scripts ( Thus I wonder how important it is to provide a CommonJS module loader. Even for main.js, perhaps it'd be better to wait for bug 1308512 - [meta] Migrate from Cu.import to ES6 Modules or some other way to load ES6 modules in that script, like the proposed import() statement. Also, note that there are some existing module loaders in Firefox already, like https://searchfox.org/mozilla-central/source/devtools/shared/Loader.jsm and https://searchfox.org/mozilla-central/source/toolkit/components/workerloader/require.js. And there are implementations like https://stuk.github.io/require1k/ that are intended for web contexts but might be adaptable to main.js.
Hmm, I'm leery of that name, as it suggests that Node modules are supported, and many of them won't be. Although it's true that NPM is used to distribute modules intended for web contexts, so if you were using NPM to install such packages for use in web contexts, as opposed to main.js, then it might make sense. But then it should be possible to use module scripts there.
Setting aside the CommonJS question, I'm unsure how best to expose qbrt-specific interfaces to applications in general. Currently, the only public interfaces that qbrt exposes are in Runtime.jsm, which also contains some private interfaces (like Runtime.start). It's also unclear to what extent qbrt will evolve its own APIs, as opposed to exposing the APIs of the underlying runtime. Nevertheless, my general inclination would be to use the best mechanism currently supported by the underlying runtime, which for pure JS module implementations is probably ES6 modules for web contexts and JSMs for main.js. (Native module implementations is a whole different issue for which I don't have any answer at all at the moment.) |
Hm. Either I misunderstand you or you understand me, or perhaps we misunderstand each other here. i e (example blatantly stolen from Electron) const {BrowserWindow} = require('qbrt');
let win = new BrowserWindow({width: 800, height: 600});
win.URL = "https://google.com";
Yea. i had been experimenting with said ES6 module loaders before. What I have found is that the current implementation acts "differently" from what one would think.
That's what my implementation is currently based on 😉 I absolutely agree that Note that whatever direction we take, we'd still have to work on the module resolution itself. Just implementing
I've seen that one before. I think there is at least one part of the Gecko codebase that already uses something similar. It's fairly simple, but not sure if it's also flexible enough for a main process1 implementation.
Yea. As I was saying, I'm not a fan either. I have that vision of a fairly unified, stable API. Something more similar to Chrome apps (or also the old Firefox Add-on SDK) than Electron, where we provide most of the SDK and allow only a limited extension mechanism. That way there would be less security issues to deal with. Less broken modules. Less complaints by users. Less off-topic questions about 1: If you don't protest, I'd like to establish the same/similar naming conventions used by Electron. Most prominently, referring to |
I understand, and I don't necessarily dislike the idea. I do have a reservation about it, though. Unlike in Electron, the runtime that qbrt uses (i.e. Firefox/Gecko) already provides basic (and advanced) functionality via the APIs it exposes through XPCOM, WebIDL, JSMs, and the Web platform. Ok, those APIs tend to be harder to use than the Electron equivalents, so I do still see value in creating new ones that abstract away the complexity. Nevertheless, I'm unsure that this should be part of qbrt itself, as opposed to a project that uses qbrt to run/package apps and provides additional APIs as part of a XUL app that it uses in place of qbrt's. Then qbrt would focus on being a packager, while the other project would focus on APIs for apps that didn't want to use the existing ones. And other projects might focus on particular use cases, like generating site-specific browsers. That said, this is weakly held opinion, and I'm open to being convinced otherwise! I've been working on a launcher executable for qbrt over in [this other repository(https://github.com/mykmelez/trbl), with the idea that it too would be a separate project that qbrt would use. But maybe it too should be part of qbrt itself. |
Yes, However, if you only need to control the order of execution of code, then you should be able to do that by putting all of it into modules, including your basic page logic, such that your about: page has a "main" module that loads (deferred) via |
Hmm, I'm pretty sure Mozilla won't reuse a require() implementation for import().
As with a node_modules directory, I'd be careful about using Electron's terminology to avoid misleading developers into thinking that the runtime that qbrt uses has the same process model. I might use "main process," which seems innocuous enough. But I'd use Gecko's "content process" terminology for the processes that Gecko creates when you load web content in a |
Sounds good to me. Guess I need to revisit the Electron architecture docs. Didn't realize the chrome inside Electron was running in the renderer process. |
I'm currently working on
Loader.jsm
, a module to implementrequire()
1.The early PoC basically already works properly, finding local modules (
require("./moduleName")
) as well as modules insidedist/qbrt/lib
, which I added for testing purposes and figured this is where the CommonJS interface brought up in #79 might live.I only spent 20 minutes working on it, so it's still rough around the edges.
Furthermore, my local qbrt fork isn't based on Firefox Nightly but on a fork of an older Gecko-Dev based around Firefox 56 (so the implementation for upstream qbrt will have to have a few lines changed. Shouldn't be a lot of work though).
Two reasons I won't share code, just yet.
That being said, I wonder if you'd be up for a brainstorming session. Would you be willing to jointly work out/standardize the shape of the loader, @mykmelez?
For starters. What do you think of
node_modules
? Should it be included inqbrt
's implementation of the CommonJS loader mechanism2 ? How shouldqbrt
(the hypothetical CommonJS interface mentioned in aforementioned issue) be exposed? As a builtin (i erequire("qbrt")
)? Or perhaps similar to the way the legacy Addon-SDK used to be exposed (require("qbrt/moduleName")
) ?1: Eventually I'd like to find a way to expose it as a global in a more direct fashion, too. Without
const { require } = Cu.import('resource://qbrt/modules/Loader.jsm', {});
inside an app'smain.js
.2: Personally, I'll say I'm not a fan of it. It seems more trouble than it's worth. But, alas, I might be missing something.
The text was updated successfully, but these errors were encountered: