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

fix: pass domains to runtime options #333

Merged
merged 4 commits into from
Jun 23, 2021
Merged

fix: pass domains to runtime options #333

merged 4 commits into from
Jun 23, 2021

Conversation

danielroe
Copy link
Member

@danielroe danielroe commented Jun 23, 2021

This no longer treats static as unique and moves domains to the global context.

closes #324

@codecov-commenter
Copy link

Codecov Report

Merging #333 (f94fbbe) into main (1d85042) will decrease coverage by 45.24%.
The diff coverage is 40.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main     #333       +/-   ##
===========================================
- Coverage   59.50%   14.25%   -45.25%     
===========================================
  Files          26       24        -2     
  Lines         600      533       -67     
  Branches      150      136       -14     
===========================================
- Hits          357       76      -281     
- Misses        243      457      +214     
Impacted Files Coverage Δ
src/module.ts 96.15% <ø> (-0.28%) ⬇️
src/provider.ts 59.45% <0.00%> (-12.77%) ⬇️
src/ipx.ts 73.07% <100.00%> (+1.07%) ⬆️
src/runtime/providers/glide.ts 0.00% <0.00%> (-100.00%) ⬇️
src/runtime/providers/imgix.ts 0.00% <0.00%> (-100.00%) ⬇️
src/runtime/providers/fastly.ts 0.00% <0.00%> (-100.00%) ⬇️
src/runtime/providers/static.ts 0.00% <0.00%> (-100.00%) ⬇️
src/runtime/providers/prismic.ts 0.00% <0.00%> (-85.72%) ⬇️
src/runtime/providers/ipx.ts 0.00% <0.00%> (-84.62%) ⬇️
src/runtime/components/image.mixin.ts 0.00% <0.00%> (-81.82%) ⬇️
... and 11 more

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 1d85042...f94fbbe. Read the comment docs.

@danielroe danielroe requested a review from pi0 June 23, 2021 12:12
@pi0
Copy link
Member

pi0 commented Jun 23, 2021

Multi-provider purpose is to allow transparently switching between them (they can yet normalize to what they expect like trimming to hostname only) For domain is there any special reason to make it vendor-specific?

Why is important? By ensuring there is one source of trust of allowed domains, we can also warn users in runtime (without provider level check) domain is not included or automatically externalize.

@pi0 pi0 added the pending label Jun 23, 2021
@pi0 pi0 removed the pending label Jun 23, 2021
@pi0 pi0 changed the title fix: default ipx, static and vercel domains options to module options fix: pass domains to runtime options Jun 23, 2021
@pi0 pi0 merged commit 33ada92 into main Jun 23, 2021
@pi0 pi0 deleted the fix/domains-option branch June 23, 2021 18:09
procrates pushed a commit to procrates/nuxt-image that referenced this pull request Feb 21, 2023
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.

IPX runtime not work with external domains
3 participants