Add a webextension preset #567

helfi92 · 2017-12-07T17:54:34Z

No description provided.

helfi92 · 2017-12-07T17:57:49Z

Feel free to use https://github.com/helfi92/webextension-neutrino-test to test this preset.

web-ext provides some commands which the current commit doesn’t handle:

lint (uses mozilla/addons-linter)
sign

For linting, should we simply create a preset for it. What about the sign command?

helfi92 · 2017-12-07T19:29:19Z

To develop with this preset, you first need toyarn build to generate the build directory. yarn start will open a firefox instance reading from the build directory. the build directory is not meant to be touched. When something is changed in the src directory, the extension on that firefox instance reloads.

helfi92 · 2017-12-07T19:33:55Z

I added react specific babel presets to support building extensions using React. Should this be included as a default? I'm not sure we should include React specifics in the preset...

eliperelman · 2017-12-07T21:39:34Z

Most of this seems like a copy of the Web preset. Can we just use web and disable the pieces we don't need?

eliperelman · 2017-12-07T21:40:09Z

For react, I think we probably don't need it. At least right now.

helfi92 · 2017-12-07T21:47:02Z

Most of this seems like a copy of the Web preset. Can we just use web and disable the pieces we don't need?

My only concern is that adding something to the web preset in the future may break the web-extension preset. If you still feel like this shouldn't be a concern, I am okay with it.

For react, I think we probably don't need it. At least right now.

Sure thing. Maybe we could add the react bit to the scaffolding part.

eliperelman · 2017-12-08T02:20:40Z

My only concern is that adding something to the web preset in the future may break the web-extension preset. If you still feel like this shouldn't be a concern, I am okay with it.

Understood, and I'm not concerned about it. If there is something that causes breakage in another preset, we should consider that to be a breaking change anyway, and something worthy of a major version bump. We shouldn't be purposefully breaking middleware.

Sure thing. Maybe we could add the react bit to the scaffolding part.

Yeah we can consider it. Maybe it would be cool if it could be composed like so, but that does assume ordering, which I am not the biggest fan of:

module.exports = {
  use: [
    '@neutrinojs/web',
    '@neutrinojs/webextension'
  ]
};

module.exports = {
  use: [
    '@neutrinojs/react',
    '@neutrinojs/webextension'
  ]
};

I guess we could say something about composition, but I still haven't figured out what a nice API for this would look like:

module.exports = {
  use: [
    ['@neutrinojs/webextension', {
      extends: '@neutrinojs/react'
    }]
  ]
};

eliperelman · 2017-12-08T02:22:08Z

As an aside, I think this will be named webextension instead of web-extension, given that they are specifically named WebExtensions.

https://developer.mozilla.org/en-US/Add-ons/WebExtensions/What_are_WebExtensions

* Renamed the preset to webextension * Deleted unwanted configs from the web preset * Store hot-update.{js, json} files in build/hot/ * Stop using hashes because it's not useful for web extensions. Also, the main html and js files need to be referenced in manifest.json and so it becomes hard to reference them if they have hashes.

helfi92 · 2017-12-08T16:49:10Z

I updated https://github.com/helfi92/webextension-neutrino-test to help test and experiment with the preset.

helfi92 · 2017-12-20T15:13:57Z

@kumar303 I am creating a preset for creating web-extensions using Neutrino. I was wondering if you could give me some feedback on the current pull-request. Thanks in advance!

kumar303

Thanks for the heads up. I took a look at the code but didn't test it. I think it needs some error handling otherwise it looks good. Feel free to ping me again if you have questions.

kumar303 · 2017-12-20T15:20:41Z

packages/webextension/index.js

+        .end()
+      .end()
+    .when(neutrino.options.command === 'start', () => {
+      webExt.cmd.run({


This returns a promise which could be rejected in some situations. I don't see .catch() handled. At the very least it should log the error because I don't think web-ext will do that in the run() function. You could test it by omitting sourceDir because that should reject the promise with some kind of error.

Agreed. I added a .catch() in my last commit.

kumar303 · 2017-12-20T15:26:09Z

packages/webextension/index.js

+    .when(neutrino.options.command === 'start', () => {
+      webExt.cmd.run({
+        noInput: true,
+        sourceDir: neutrino.options.output


If this directory contains any unimportant build artifacts then you should consider passing an ignoreFiles array to run().

run() will start a file watcher within sourceDir. When a file gets touched, it will reload the extension unless the file matches any pattern in ignoreFiles.

It's important that the web-ext file watcher does not look for changes in the actual source files. It should only look for changes in the final executable files that are built from the source (I think that's how you have it configured). This is because if web-ext starts to reload the extension when a source file changes, it might reload too soon, before the final files are built by babel/webpack/whatever.

kumar303 · 2017-12-20T15:32:20Z

packages/webextension/index.js

+        .end()
+      .end()
+    .when(neutrino.options.command === 'start', () => {
+      webExt.cmd.run({


I don't know much about neutrino but is there a way to make some run() parameters configurable by the user? I can see a couple parameters here that the user may want to configure such as firefox (path to executable), firefoxProfile, browserConsole, pref...

Yep. I made all the options configurable in my latest commit. That was a good point!

eliperelman

So far so good.

Do you think we should perform some extra magic by reading and doing some things if a user provides a manifest.json?

Also, we will need documentation around this, and how it works with the manifest.json.

eliperelman · 2018-01-15T03:47:40Z

packages/webextension/package.json

+  "repository": "https://github.com/mozilla-neutrino/neutrino-dev/tree/master/packages/webextension",
+  "homepage": "https://neutrino.js.org",
+  "bugs": "https://github.com/mozilla-neutrino/neutrino-dev/issues",
+  "dependencies": {


We will need the publishConfig and engines copied here.

eliperelman · 2018-01-15T03:54:46Z

packages/webextension/index.js

+        .end()
+      .end()
+    .when(neutrino.options.command === 'start', () => {
+      webExt.cmd


I see where we are going with this, but exiting the process from here is going to skip all the error handling we have in the API.

What if we took this part out of this conditional and use an event?

neutrino.on('start', () => webExt.cmd.run(options.webExtRun, { shouldExitProgram: false }));

This way would mean we could skip handling the promise altogether and let the API handle it. Give it a shot and let me know if that works.

Morikko · 2018-03-22T16:48:12Z

I just checked, it looks promising.

However a few questions:

Is it mandatory to do neutrino build before neutrino start. If I did only the second command, the copying files were missing (e.g. manifest.json...)
The problem with question 1 raised a new problem. If you modify a copying file (eg: popup html), the file is not copy again to the build folder. You have to manually do again neutrino build.
For this wouldn't be better to split the folder used by neutrino start and neutrino build. The latter could use another folder where the build is optimized for production.

Do you think we should perform some extra magic by reading and doing some things if a user provides a manifest.json?

What do you mean ?
Are you thinking about finding the different entries from the manifest ?

Also allowing web-ext to read a config.js file would be nice to let the user choose the profile, firefox executable and so on.

Finally, a command like neutrino release would be useful for generating the .xpi (for FF) and .zip (for chrome) from the optimized build.

eliperelman · 2018-03-22T16:57:03Z

For this wouldn't be better to split the folder used by neutrino start

My preference would be for the start command to build files to a temp directory, or in memory.

What do you mean ?

For example, if the user sets the popup file in the manifest.json, should we automatically add it to Neutrino's mains?

Are you thinking about finding the different entries from the manifest ?

Yes, that's the gist of it. :)

Also allowing web-ext to read a config.js file would be nice to let the user choose the profile, firefox executable and so on.

We would like this to be driven by settings in .neutrinorc.js, just so it's consistent with every other way of configuration across presets/middleware.

Finally, a command like neutrino release would be useful for generating the .xpi (for FF) and .zip (for chrome) from the optimized build.

Could this happen in the build command?

kumar303 · 2018-03-22T17:01:42Z

Also allowing web-ext to read a config.js file would be nice to let the user choose the profile, firefox executable and so on.

Aside from hooking into the standard ways of configuring neutrino presets, I should mention that web-ext now supports standalone config files. The config feature was added after this PR began. A webExt key in package.json is also supported but this isn't documented yet (FYI).

Morikko · 2018-03-22T17:09:22Z

My preference would be for the start command to build files to a temp directory, or in memory.

Make sense, it respects the standard behavior.

We would like this to be driven by settings in .neutrinorc.js, just so it's consistent with every other way of configuration across presets/middleware.

@kumar303 said what I was thinking for. Everything is already provided by web-ext and would avoid to bind too much .neutrinorc.js to web-ext.

Could this happen in the build command?

Yes, it would avoid new commands.

@helfi92 do you have some plan to go ahead ?
I want to start bundling my extension, so I am really interested :)

@kumar303 What about chome/chromium support ?
Because using web-ext in this preset breaks the flow for chrome (only) extension...

eliperelman · 2018-03-22T17:17:17Z

Everything is already provided by web-ext and would avoid to bind too much .neutrinorc.js to web-ext.

There is a reason for this. By managing this in .neutrinorc.js, it can also be managed by an external package and consumed by Neutrino. This way web-ext settings can be composed across projects, without having to add boilerplate to every project.

do you have some plan to go ahead ?
I want to start bundling my extension, so I am really interested :)

@helfi92 will be out for the next week or so, so if you want to take this PR over the finish line with the suggestions, we would be happy to have your contribution!

kumar303 · 2018-03-22T17:54:59Z

What about chome/chromium support

This feature is being tracked in mozilla/web-ext#809 and we may have someone to work on it soon but no concrete plans.

Morikko · 2018-03-27T11:05:28Z

@eliperelman One question about the PR workflow.
I would like to use the last version of neutrino to continue the work. I have cloned the PR locally.
Can I rebase it (Commit hash is changed) or should I do a merge to master?

So, after my commits will be added on top of helfi92's ones in this PR.

eliperelman · 2018-03-27T11:50:05Z

I would recommend rebasing against master. If you need a guide of how to do a rebase, I would recommend giving this a read:

http://blessing.io/git/git-rebase/open-source/2015/08/23/git-rebasing-to-resolve-conflicts.html

Morikko · 2018-03-27T15:07:50Z

In neutrino by adding an entry in mains, can you compile only a JS without an HTML ?

eliperelman · 2018-03-27T15:23:53Z

@Morikko I don't understand the question, could you rephrase?

Morikko · 2018-03-27T15:42:20Z

UPDATE:

background.js and content scripts do not need an HTML page so neutrino should create only a JavaScript file
I found how to do this

Morikko · 2018-03-28T15:49:40Z

I have commited new things but it is visible only on my local repository.
How do I continue on this PR ? (link the commit here)
Isn't it rebase the problem ? I still have the helfi92's commits but they have new hashes.

In brief:

Update to neutrino v8
Split folder used by start/build commands
Files to copy are also watched
Some work to use custom options for web-ext command from .neutrinorc.js

Also I am using this extension to test it.

Morikko · 2018-03-28T15:51:27Z

@eliperelman Do neutrino has an event for when a command ends ?

Like

api.on('end-start', () => {
  // ...
});

Morikko · 2018-03-30T23:41:24Z

Few updates:

Use web-ext lint on start/build/lint
Read Manifest for entries

@eliperelman On build, a chunk optimization create a runtime.js. For HTML files, it is added with html-template. I don't know how to add chunks directly to JavaScript file. As a consequence, all JavaScript files crash on start because they don't find the resources in runtime.js.

Note: JavaScript files are background and content scripts.

eliperelman · 2018-04-02T13:25:33Z

Do neutrino has an event for when a command ends ?

Yes, there is prestart, start, prebuild, build, and prerun, run.

https://neutrino.js.org/api/#events

eliperelman · 2018-05-23T21:31:57Z

See #831 for next steps here, or if you think we should just close this at the moment.

edmorley · 2018-06-08T08:51:36Z

Closing this in favour of #831.

helfi92 self-assigned this Dec 7, 2017

helfi92 requested a review from eliperelman December 7, 2017 19:23

eliperelman mentioned this pull request Dec 8, 2017

WebExtension preset #306

Closed

helfi92 added 2 commits December 8, 2017 11:47

Add webextension preset

b24203b

helfi92 force-pushed the webextensions branch from bfaa738 to 05bdce4 Compare December 8, 2017 16:47

helfi92 changed the title ~~Add a web-extension preset~~ Add a webextension preset Dec 10, 2017

kumar303 reviewed Dec 20, 2017

View reviewed changes

Handle promise rejection, and make run opts configurable

cb6f946

eliperelman added the feature label Dec 21, 2017

eliperelman suggested changes Jan 15, 2018

View reviewed changes

Morikko mentioned this pull request Apr 29, 2018

Web Extension Preset #831

Closed

edmorley closed this Jun 8, 2018

Add a webextension preset #567

Add a webextension preset #567

Conversation

helfi92 commented Dec 7, 2017

helfi92 commented Dec 7, 2017 • edited Loading

helfi92 commented Dec 7, 2017 • edited Loading

helfi92 commented Dec 7, 2017

eliperelman commented Dec 7, 2017

eliperelman commented Dec 7, 2017

helfi92 commented Dec 7, 2017 • edited Loading

eliperelman commented Dec 8, 2017

eliperelman commented Dec 8, 2017

helfi92 commented Dec 8, 2017

helfi92 commented Dec 20, 2017

kumar303 left a comment

Choose a reason for hiding this comment

kumar303 Dec 20, 2017

Choose a reason for hiding this comment

helfi92 Dec 20, 2017

Choose a reason for hiding this comment

kumar303 Dec 20, 2017

Choose a reason for hiding this comment

kumar303 Dec 20, 2017

Choose a reason for hiding this comment

helfi92 Dec 20, 2017

Choose a reason for hiding this comment

eliperelman left a comment

Choose a reason for hiding this comment

eliperelman Jan 15, 2018

Choose a reason for hiding this comment

eliperelman Jan 15, 2018

Choose a reason for hiding this comment

Morikko commented Mar 22, 2018

eliperelman commented Mar 22, 2018 • edited Loading

kumar303 commented Mar 22, 2018

Morikko commented Mar 22, 2018

eliperelman commented Mar 22, 2018

kumar303 commented Mar 22, 2018

Morikko commented Mar 27, 2018

eliperelman commented Mar 27, 2018

Morikko commented Mar 27, 2018

eliperelman commented Mar 27, 2018

Morikko commented Mar 27, 2018 • edited Loading

Morikko commented Mar 28, 2018

Morikko commented Mar 28, 2018

Morikko commented Mar 30, 2018

eliperelman commented Apr 2, 2018

eliperelman commented May 23, 2018

edmorley commented Jun 8, 2018

helfi92 commented Dec 7, 2017 •

edited

Loading

helfi92 commented Dec 7, 2017 •

edited

Loading

helfi92 commented Dec 7, 2017 •

edited

Loading

eliperelman commented Mar 22, 2018 •

edited

Loading

Morikko commented Mar 27, 2018 •

edited

Loading