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

Various import/copy cleanup, avoid unnecessary library injection #3511

Merged
merged 8 commits into from
May 30, 2022

Conversation

fregante
Copy link
Collaborator

@fregante fregante commented May 29, 2022

A few small changes:

  • Use browser.* APIs whenever possible (a previous bug has been fixed)
  • Avoid triplicate injection libraries (seen while debugging The content script is injected twice (editor open, click browser action) #3510)
  • Drops a warning we'll never fix
  • Faster testMatchPatterns by converting one pattern at a time + add tests
  • Fix LoggingSettings console error (<p> in <p>) that appears in the Options page
  • Fix (?) Ember code
  • Other minor cleanups and typos

Before

❯ npm ls webext-content-scripts
@pixiebrix/extension@1.6.4-alpha.1
├── webext-content-scripts@1.0.1 <-- version A
├─┬ webext-dynamic-content-scripts@8.1.0 
│ ├─┬ content-scripts-register-polyfill@3.2.0
│ │ └── webext-content-scripts@0.12.0 <-- version B
│ └── webext-content-scripts@0.12.0 <-- version B but somehow still not "deduped"
└─┬ webext-tools@0.3.0
  └── webext-content-scripts@0.12.0

After

❯ npm ls webext-content-scripts
@pixiebrix/extension@1.6.4-alpha.1
├── webext-content-scripts@1.0.1
├─┬ webext-dynamic-content-scripts@8.1.1
│ ├─┬ content-scripts-register-polyfill@3.2.2
│ │ └── webext-content-scripts@1.0.1 deduped
│ └── webext-content-scripts@1.0.1 deduped
└─┬ webext-tools@1.0.0
  └── webext-content-scripts@1.0.1 deduped

Fixed via:

@codecov-commenter
Copy link

codecov-commenter commented May 29, 2022

Codecov Report

Merging #3511 (db82b64) into main (33cbffb) will increase coverage by 0.01%.
The diff coverage is 40.00%.

@@            Coverage Diff             @@
##             main    #3511      +/-   ##
==========================================
+ Coverage   40.34%   40.35%   +0.01%     
==========================================
  Files         769      769              
  Lines       22407    22401       -6     
  Branches     4711     4709       -2     
==========================================
  Hits         9040     9040              
+ Misses      12449    12443       -6     
  Partials      918      918              
Impacted Files Coverage Δ
src/background/activateBrowserActionIcon.ts 0.00% <0.00%> (ø)
src/background/permissionPrompt.ts 0.00% <0.00%> (ø)
src/development/autoreload.ts 0.00% <0.00%> (ø)
src/frameworks/contrib/ember.ts 0.00% <0.00%> (ø)
src/layout/EnvironmentBanner.tsx 0.00% <0.00%> (ø)
src/options/pages/settings/LoggingSettings.tsx 0.00% <ø> (ø)
src/sidebar/messenger/api.ts 100.00% <ø> (+14.28%) ⬆️
src/blocks/available.ts 76.36% <100.00%> (+6.72%) ⬆️
src/chrome.ts 57.14% <0.00%> (-1.79%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 33cbffb...db82b64. Read the comment docs.

@fregante fregante marked this pull request as ready for review May 30, 2022 18:28
// As of Ember 4.6.0 this is not a real Symbol, so Object.keys should detect it. If they change it, this will stop working:
// https://github.com/emberjs/ember.js/blob/55ffe6326f11efcaeb278cdf7e0f86543daa9f04/packages/%40ember/-internals/utils/lib/symbol.ts#L14-L26
// https://github.com/emberjs/ember.js/blob/55ffe6326f11efcaeb278cdf7e0f86543daa9f04/packages/%40ember/-internals/views/lib/compat/attrs.ts#L1
return Object.keys(cell).some((key) => key.startsWith("__MUTABLE_CELL"));
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@twschiller twschiller self-requested a review May 30, 2022 21:14
@twschiller twschiller changed the title Small fixes Various import/copy cleanup, avoid unnecessary library injection May 30, 2022
@twschiller twschiller merged commit e56e64c into main May 30, 2022
@twschiller twschiller deleted the F/lint/cleanup branch May 30, 2022 21:18
@twschiller twschiller added this to the 1.6.4 milestone May 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants