Skip to content
This repository has been archived by the owner on Apr 27, 2023. It is now read-only.

Create a developer mode command to ease development #295

Merged
merged 2 commits into from
Mar 16, 2021

Conversation

Dexterp37
Copy link
Contributor

@Dexterp37 Dexterp37 commented Jan 12, 2021

This introduces a 'watch' command. The command uses rollup to watch for UI and addon changes, updates the packed files, and makes sure web-ext picks up the changes. This fixes #3.

Unfortunately, if the rally control panel (addon options page) is open, that specific page does not get refreshed. User must refresh it manually or close/reopen that tab. That's due to a bug in Firefox.

Checklist for reviewer:

  • The description should reference a bug or github issue, if relevant.
  • There must be a CHANGELOG.md entry for any non-test change.
  • Any change to the NPM commands must be carefully reviewed to make sure it won't break the Add-ons pipeline.
  • Any version increase must follow the release process.

@Dexterp37 Dexterp37 self-assigned this Jan 12, 2021
@Dexterp37
Copy link
Contributor Author

Unfortunately, the approach above doesn't work due to mozilla/web-ext#2122

@rpl
Copy link

rpl commented Jan 13, 2021

Unfortunately, the approach above doesn't work due to mozilla/web-ext#2122

@Dexterp37 I just closed mozilla/web-ext#2122, but only because it was a duplicate of mozilla/web-ext#2104, it is a valid issue and (as we agreed in our triage meeting today) supporting --watch-file used more than ones is our preferred approach to fix that issue (instead of just improving the error message).

This adds `npm run watch` to allow developers running a
single command, build the ui and the addon and automatically
watch for changes (and live reload it in the browser).
@Dexterp37 Dexterp37 marked this pull request as ready for review March 12, 2021 15:25
@Dexterp37 Dexterp37 changed the title [WIP] Create a developer mode command to ease development Create a developer mode command to ease development Mar 12, 2021
@Dexterp37 Dexterp37 requested a review from rhelmer March 12, 2021 15:27
Copy link
Contributor

@rhelmer rhelmer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that this is printed on the console:

  Your application is ready~! 🚀

  - Local:      http://localhost:5000
  - Network:    Add `--host` to expose

However that link doesn't work since this page is not in moz-extension://, so the polyfill library throws:

Uncaught Error: This script should only be loaded in a browser extension.

Related is that Svelte does not auto-reload the UI when loaded in moz-extension://, presumably because the livereloadscript thing it injects into the bundle doesn't work here.

So I'd ask that we suppress that message about the localhost server being ready, because that will just confuse new developers.

I think that it might be possible to make livereload actually work in extension context, but this feels unimportant compared to the overall greatness of this change :) Something to look into in the future maybe.

@rhelmer
Copy link
Contributor

rhelmer commented Mar 16, 2021

Related is that Svelte does not auto-reload the UI when loaded in moz-extension://, presumably because the livereloadscript thing it injects into the bundle doesn't work here.

Ah yeah this happens when it tries in moz-extension://:

Loading failed for the <script> with source “moz-extension://d32eeb5a-8ad4-2c47-91f5-b65a7f45ee9d:35729/livereload.js?snipver=1”.

@rhelmer
Copy link
Contributor

rhelmer commented Mar 16, 2021

Unfortunately, if the rally control panel (addon options page) is open, that specific page does not get refreshed. User must refresh it manually or close/reopen that tab. That's due to a bug in Firefox.

Hm, are you certain this is the cause? If it were that bug, I'd think that the options page would close out from under you. I think the add-on being loaded as temporary by web-ext is avoiding that bug actually and lack odf auto-refresh is #295 (comment)

However as I said in the review comment I am totally down with deferring all of this until later :)

@Dexterp37
Copy link
Contributor Author

Related is that Svelte does not auto-reload the UI when loaded in moz-extension://, presumably because the livereloadscript thing it injects into the bundle doesn't work here.

Yep, I'm aware that isn't working. If it weren't for the bug mentioned in the PR description, it would still work: web-ext reloads the extension, but Firefox is not closing the option window and re-opening it (it should do that).

@rhelmer
Copy link
Contributor

rhelmer commented Mar 16, 2021

Related is that Svelte does not auto-reload the UI when loaded in moz-extension://, presumably because the livereloadscript thing it injects into the bundle doesn't work here.

Yep, I'm aware that isn't working. If it weren't for the bug mentioned in the PR description, it would still work: web-ext reloads the extension, but Firefox is not closing the option window and re-opening it (it should do that).

Would you mind splitting this out to a separate issue? It needn't block this one :)

(edit: I think you are right about the fix for that Firefox bug helping us if the fix is to reload the options page automatically, I was confusing it with this bug)

@Dexterp37 Dexterp37 requested a review from rhelmer March 16, 2021 17:35
@Dexterp37
Copy link
Contributor Author

Related is that Svelte does not auto-reload the UI when loaded in moz-extension://, presumably because the livereloadscript thing it injects into the bundle doesn't work here.

Yep, I'm aware that isn't working. If it weren't for the bug mentioned in the PR description, it would still work: web-ext reloads the extension, but Firefox is not closing the option window and re-opening it (it should do that).

Would you mind splitting this out to a separate issue? It needn't block this one :)

Filed #498

@Dexterp37 Dexterp37 merged commit fe33dae into mozilla-rally:master Mar 16, 2021
@Dexterp37 Dexterp37 deleted the fix_dev_cmd branch March 16, 2021 17:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

configure web-ext to refresh if the Svelte bundle updates
3 participants