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

chore(deps): bump webextension-store-meta from 1.1.0 to 1.2.1; test [chromewebstore] #10094

Merged

Conversation

dependabot[bot]
Copy link
Contributor

@dependabot dependabot bot commented on behalf of github Apr 17, 2024

Bumps webextension-store-meta from 1.1.0 to 1.2.1.

Changelog

Sourced from webextension-store-meta's changelog.

[1.2.1] - 2024-04-18

Updated

  • Loosen engines.node constraint #5

[1.2.0] - 2024-04-10

Updated

  • Implement with TypeScript.

Fixed

  • Fix #2 UTF-8 location header causing infinite redirect loop.
Commits

Dependabot compatibility score

You can trigger a rebase of this PR by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot show <dependency name> ignore conditions will show all of the ignore conditions of the specified dependency
  • @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

Bumps [webextension-store-meta](https://github.com/awesome-webextension/webextension-store-meta) from 1.1.0 to 1.2.1.
- [Changelog](https://github.com/awesome-webextension/webextension-store-meta/blob/main/CHANGELOG.md)
- [Commits](https://github.com/awesome-webextension/webextension-store-meta/commits/v1.2.1)

---
updated-dependencies:
- dependency-name: webextension-store-meta
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
@dependabot dependabot bot added dependencies Related to dependency updates javascript Pull requests that update Javascript code labels Apr 17, 2024
Copy link
Contributor

🚀 Updated review app: https://pr-10094-badges-shields.fly.dev

@chris48s chris48s changed the title chore(deps): bump webextension-store-meta from 1.1.0 to 1.2.1 chore(deps): bump webextension-store-meta from 1.1.0 to 1.2.1; test [chromewebstore] Apr 17, 2024
@xmcp
Copy link

xmcp commented Apr 17, 2024

@chris48s
Copy link
Member

This version is at least installable, but breaks all chrome web store badges.

Failure is TypeError: ChromeWebStore.load is not a function here:

return await ChromeWebStore.load({ id: storeId })

There are no documented changes to the public API. We are using the library as documented. I'm out of time today. I'll come back to it tomorrow, but it looks like the BC break is this commit: awesome-webextension/webextension-store-meta@0565f15

@xmcp
Copy link

xmcp commented Apr 17, 2024

Thanks for the investigation. I can further diagnose this issue and report to upstream.

@xmcp
Copy link

xmcp commented Apr 18, 2024

There are no documented changes to the public API

Well, the usage example in README did change to reflect the new API signature. It's not obvious to existing users though.

image

After discussing with the upstream, they decided to keep this breaking change and document it in CHANGELOG.

Changing import ChromeWebStore from ... to import { ChromeWebStore } from ... in chrome-web-store-base.js should do the migration.

I don't love this, but it will do
Copy link
Contributor

Messages
📖

✨ Thanks for your contribution to Shields, @dependabot[bot]!

Generated by 🚫 dangerJS against c0ef49c

Copy link
Contributor

🚀 Updated review app: https://pr-10094-badges-shields.fly.dev

@chris48s
Copy link
Member

OK. There shouldn't really be breaking changes in a minor version, but thanks for following it up.

Comment on lines 15 to +27
// Keep this "inaccessible" test, since this service does not use BaseService#_request.
const mockAgent = new MockAgent()
t.create('Version (inaccessible)')
.get('/alhjnofcnnpeaphgeakdhkebafjcpeae.json')
.networkOff()
// webextension-store-meta uses undici internally, so we can't mock it with nock
.before(function () {
setGlobalDispatcher(mockAgent)
mockAgent.disableNetConnect()
})
.after(async function () {
await mockAgent.close()
setGlobalDispatcher(new Agent())
})
Copy link
Member

@chris48s chris48s Apr 18, 2024

Choose a reason for hiding this comment

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

Just for code archaeology purposes:

Uniquely, the chrome web store integration bypasses our usual HTTP client. We farm out making the request to an external library.

In v1.2.0, webextension-store-meta switched their HTTP client from node-fetch to undici.

In theory, that is just internals not API surface but because undici bypasses the normal node HTTP stack, this means we now can't mock the requests it makes using nock (see nock/nock#2183 ). Oh no - they removed spacebar heating!

For the purposes of just getting these tests to work, I've implemented an ad-hoc mocking solution for undici and copy/pasted it to the 3 places we need to do this. If we had a lot of code using undici, I'd pull this out to an abstraction but given this is a uniquely non-standard service it is probably actively unhelpful to implement this on the core test helper. We can live with this.

Copy link

Choose a reason for hiding this comment

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

Yep, it intentionally bypasses the node http module because it has problems handling non-ascii location headers sent by Google's server.

@chris48s chris48s added this pull request to the merge queue Apr 18, 2024
Merged via the queue into master with commit 68bfc3c Apr 18, 2024
21 checks passed
@chris48s chris48s deleted the dependabot/npm_and_yarn/webextension-store-meta-1.2.1 branch April 18, 2024 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Related to dependency updates javascript Pull requests that update Javascript code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants