Skip to content
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

Add parameter "--watch-file" to allow reloading after build is done #1626

Closed
Juraj-Masiar opened this issue Jun 3, 2019 · 8 comments · Fixed by #1784
Closed

Add parameter "--watch-file" to allow reloading after build is done #1626

Juraj-Masiar opened this issue Jun 3, 2019 · 8 comments · Fixed by #1784

Comments

@Juraj-Masiar
Copy link

Is this a feature request or a bug?

Feature

What is the current behavior?

web-ext will try to reload add-on too soon. If you are using some builder, this will often result in partially loaded add-on as your sources are only being build.
See the related bug that tries to solve the save problem with a delay parameter: #1573

What is the expected or desired behavior?

As @Rob--W suggested: #1573 (comment) one solution would be to watch only single file, that would be modified last when build is done, for example some "build.version".
Even though it requires changes in your bundler, it's still easy to do and it's reactive - so it will save time and your CPU as web-ext will not try to reload not fully build add-on.

@rpl
Copy link
Member

rpl commented Sep 12, 2019

@Rob--W will mentor this.

@rpl
Copy link
Member

rpl commented Sep 18, 2019

@Rob--W would you mind to add a comment with some pointers and details about what should be implemented? Thanks!

@Rob--W
Copy link
Member

Rob--W commented Sep 26, 2019

Currently, the watching feature (of web-ext run) watches a directory and all of the files within. I suggest to resolve this feature request by allowing the user to change the directory, from sourceDir to the location as given via the command line.

The directory ends up being used here:

watcher.watch([], [sourceDir], Date.now());

Previously, I described how to read the commandline argument, and forward it all the way down. While the final details are different, the start should still be accurate.
#1573 (comment)

@Shivansh2407
Copy link

Please I want to contribute to this , so can you explain me the issue and how to implement this

@Rob--W
Copy link
Member

Rob--W commented Oct 7, 2019

@Shivansh2407 See the first comment at the top of this issue, the explanation at #1626 (comment) and the link to the other comment from that comment.

@jezhou
Copy link
Contributor

jezhou commented Dec 12, 2019

@Rob--W I would love to take a stab at this if it's up for grabs.

If I understand the problem correctly, bundlers like Webpack will produce code that won't be completely hot swapped into web-ext (too eager?). This change will add an option that will only hot swap / reload the extension if a single watched file is changed (which can be "touched" in whatever bundler/builder a developer is using after the builder is done, will probably require configuration on the developer's part).

If this understanding is good enough, I can try to whip up a PR and maybe ask you some questions along the way.

It seems like this solution would also indirectly solve these issues as well?

@Juraj-Masiar
Copy link
Author

Actually I would like to note something here.
When I was reporting this here, at that time I was using custom build script that was building my extension and during the process the extension was incorrectly reloaded.

However ever since I moved to Webpack, I have no longer this problem. The Webpack is swapping build sources only when the whole build is done and it's seems to be fast enough to not cause this issue anymore.

@Rob--W
Copy link
Member

Rob--W commented Dec 12, 2019

@jezhou Up for grabs. Your understanding is about correct; this feature request addresses the general problem of reloading too eagerly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants