Made tab reload optional - sometimes we have our own code to reinject…

[mikob]

… content script necessities, or we don't have content script, and sometimes we don't want the page to refresh

[xpl]

@mikob I wonder if we can read manifest.json and pull custom options from there instead of passing them down via a function call? I don't know if it's possible, but if it is, wouldn't it be a nicer option?

[mikob]

@xpl Why would that be nicer? That would be surprising IMO. I think it's best to follow standard, unsurprising practices and let options be passed in the same way they are with most other modules. I also think that the CRX reloading isn't closely tied with what's specified in manifest.json. eg. in my case I need to exclude hot-updated files (eg. the extension options page) from reloading the CRX because those are already injected and I don't want the options page to close as it does automatically when the CRX is reloaded.

[xpl]

Why would that be nicer?

Because it would maintain the backward compatibility with the current "zero-configuration" version.

In the proposed new version users are supposed to explicitly pass the configuration to start watching files. That requires making changes to the code to make it work, right?

[xpl]

Also, putting scripts/plugins options to manifest files is not unusual. For example, it is norm with package.json – often toolchain scripts read their (custom) options from there, and it's rather convenient because it keeps configuration away from the code.

Of course, providing the ability to configure from the code is also convenient, because it would allow to dynamically create those options in case if it's needed. So maybe it is good to support both ways, I dunno.

[xpl]

Could you also please combine all your PRs into a single one? It would be easier to review and edit it then. They all seem linked and also all of them seem useful (there were prior requests to add file inclusion/exclusion), so if we are going to add that functionality, better add it all at once.

[mikob]

@xpl yes, it would be nice to have backwards compatibility but I think these changes warrant a major version bump/compatibility break? Configuration in package.json is usually for toolchain related stuff, as you said. Tools that can be configured in package.json like tslint, ava, tsconfig are never instantiated in user-land code so it makes sense that they configure in an external file. This library needs to be instantiated in userland... so it makes more sense that the configuration goes along with it when it's instantiated. If this were a webpack plugin, or otherwise didn't live in userland code, I would agree that configuration should be separated from userland code.

Could you also please combine all your PRs into a single one? It would be easier to review and edit it then. They all seem linked and also all of them seem useful (there were prior requests to add file inclusion/exclusion), so if we are going to add that functionality, better add it all at once.

You can see them all together in the final PR I made.

Lastly, I don't mean this in any passive-aggressive way, but just so you're not waiting around for it, I won't be making the changes to this PR. So someone else would need to pick up the slack if you're still unsatisfied.