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.

Sign up for GitHub

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

Jump to bottom

Add multiple cdn v2 #10189

Merged
merged 36 commits into from
Mar 8, 2024
Merged

Add multiple cdn v2 #10189

merged 36 commits into from
Mar 8, 2024

Conversation

peng
Copy link
Contributor

@peng peng commented Feb 22, 2024 •
edited
Loading

Changes

  • What does this change?
    Add build.assetsPrefix is also Object.

I also think build.assetsPrefix is also Object is good.

Previous PR: #9898

Testing

Add packages/astro/test/astro-assets-prefix-mult-cdn.test.js file for test add multiple cdn option.

Docs

Docs add how to use build.assetsPrefix with object Record<string, string> type.

Docs PR: withastro/docs#7183

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Feb 22, 2024 •
edited
Loading

🦋 Changeset detected

Latest commit: 2371fb7

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added pkg: astro Related to the core `astro` package (scope) pr: docs A PR that includes documentation for review labels Feb 22, 2024
@peng peng mentioned this pull request Feb 22, 2024
Closed
ematipico
ematipico requested changes Feb 23, 2024
View reviewed changes
packages/astro/src/@types/astro.ts Show resolved Hide resolved
packages/astro/src/core/create-vite.ts Show resolved Hide resolved
packages/astro/src/core/config/schema.ts Outdated Show resolved Hide resolved
sarah11918
sarah11918 previously requested changes Feb 23, 2024
View reviewed changes
Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Just putting a "docs block" on this one because the documentation changes are needed to the config reference source file in this repo, not in a separate docs PR.

I will address the content of the docs material in withastro/docs#6982 initially, but in fact those changes will need to be made to https://github.com/withastro/astro/blob/main/packages/astro/src/%40types/astro.ts and that Docs PR will be closed.

@sarah11918
Copy link
Member

DOCS UPDATE:

The docs content has been initially reviewed in the docs PR, and is ready to be added here as an edit to types.ts. It may still require more editing (and technical review from devs evaluating this PR), but it is a good format to now bring from the other Docs PR to here to continue working on it.

@peng
change docs
47b3d5f
@peng
replace getFileExtension with node path.extname
3a1dcdb
@peng
Adapt node extname
acd1ec9
@peng
Merge branch 'main' into add-multiple-cdn-v2
0600b34 
* main: (327 commits)
  [ci] format
  fix(node): listen on 0.0.0.0 if server.host is set to true (withastro#10282)
  [ci] format
  fix(dev): cosider `base` when special-casing `/_image` (withastro#10274)
  [ci] format
  update login flow to support Brave (withastro#10258)
  [ci] format
  improve link command (withastro#10257)
  Updates deprecated Node.js 16 github actions (withastro#10270)
  Fix Vitest check fail again (withastro#10266)
  [ci] format
  Adds auto completion of `astro:` events when adding or removing event listeners on `document` (withastro#10263)
  Update Vite to latest (withastro#10259)
  [ci] release (withastro#10236)
  [ci] format
  fix(i18n): localised index pages are overwritten (withastro#10250)
  fix: change strategy for route caching (withastro#10248)
  Fix TypeScript type definitions for `Code` component (withastro#10251)
  chang changeset (withastro#10253)
  Removes morph animations when setting transition:animate=none (withastro#10247)
  ...
@peng
multiple image test
99f53f7
@peng
Merge branch 'main' into add-multiple-cdn-v2
ecd9414 
* main:
  [ci] format
  fix(withastro#8625): smooth scrolling in SPA mode on iOS (withastro#10235)
  [ci] release (withastro#10292)
  [ci] format
  finalize WIP API (withastro#10280)
  [ci] format
  fix(markdoc & mdx): Proxy crimes (withastro#10278)
  [ci] release (withastro#10286)
  fix(audits): Don't warn about loading on data URIs (withastro#10275)
  fix(node): Safely create requests (withastro#10285)
  [ci] release (withastro#10265)
  [ci] format
  fix(assets): Solidify Node endpoint (withastro#10284)
@peng
wip space
7e6467d
@peng
update docs
30b2cd7
@peng
update docs, assetsPrefix type
3f769fa
@peng
update docs
91ea28e
@peng
update docs
de65e0a
@peng peng mentioned this pull request Mar 4, 2024
Closed
@ematipico
Copy link
Member

Heads up @peng and @bluwy

I pushed a commit that applies the following changes:

  • refines the schema to have fallback as a mandatory key inside the object
  • renames defaultSchemaPrefix to fallback

@ematipico
Copy link
Member

When will it be merged?

Monday

@Princesseuh Princesseuh added this to the 4.5.0 milestone Mar 6, 2024
bluwy
bluwy requested changes Mar 6, 2024
View reviewed changes
packages/astro/src/@types/astro.ts Outdated Show resolved Hide resolved
packages/astro/src/@types/astro.ts Outdated Show resolved Hide resolved
packages/astro/src/assets/utils/transformToPath.ts Outdated Show resolved Hide resolved
packages/astro/src/assets/utils/transformToPath.ts Outdated Show resolved Hide resolved
sarah11918
sarah11918 reviewed Mar 6, 2024
View reviewed changes
Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

I know we're all in this file at the same time, so here are some suggestions from me to consider for the docs!

packages/astro/src/@types/astro.ts Show resolved Hide resolved
packages/astro/src/@types/astro.ts Outdated Show resolved Hide resolved
packages/astro/src/@types/astro.ts Outdated Show resolved Hide resolved
packages/astro/src/@types/astro.ts Outdated Show resolved Hide resolved
@sarah11918 sarah11918 dismissed their stale review March 6, 2024 15:03

removing my block because the docs are here, being worked on!

ematipico and others added 2 commits March 7, 2024 09:51
@ematipico @sarah11918
Update packages/astro/src/@types/astro.ts
eb5eec9 
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
@ematipico @sarah11918
Apply suggestions from code review
b36026e 
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
@ematipico ematipico requested a review from bluwy March 7, 2024 09:56
@bluwy
Copy link
Member

bluwy commented Mar 7, 2024

@ematipico It doesn't look like my comments above have been addressed yet.

@ematipico
Copy link
Member

@ematipico It doesn't look like my comments above have been addressed yet.

Which ones? Sorry, it's becoming confusing 😅 I would be grateful if you can tell me what I have need to address

bluwy
bluwy approved these changes Mar 7, 2024
View reviewed changes
Copy link
Member

@bluwy bluwy left a comment

Choose a reason for hiding this comment

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

Thanks for picking this up Ema!

sarah11918
sarah11918 reviewed Mar 7, 2024
View reviewed changes
Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Looks great! Just some suggestions where I think we can use more user-friendly language in the changeset. See what you think!

.changeset/fluffy-readers-add.md Outdated Show resolved Hide resolved
.changeset/fluffy-readers-add.md Outdated Show resolved Hide resolved
.changeset/fluffy-readers-add.md Outdated Show resolved Hide resolved
@Princesseuh @sarah11918
Apply suggestions from code review
112d50b 
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
sarah11918
sarah11918 approved these changes Mar 7, 2024
View reviewed changes
Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Approving for docs!

@bluwy bluwy mentioned this pull request Mar 8, 2024
Merged
@bluwy bluwy added the semver: minor Change triggers a `minor` release label Mar 8, 2024
@ematipico ematipico merged commit 1ea0a25 into withastro:main Mar 8, 2024
14 checks passed
@astrobot-houston astrobot-houston mentioned this pull request Mar 8, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@VoxelMC VoxelMC VoxelMC left review comments

@ematipico ematipico ematipico approved these changes

@sarah11918 sarah11918 sarah11918 approved these changes

@bluwy bluwy bluwy approved these changes

Assignees
No one assigned
Labels
pkg: astro Related to the core `astro` package (scope) pr: docs A PR that includes documentation for review semver: minor Change triggers a `minor` release
Projects
None yet
Milestone
4.5.0
Development

Successfully merging this pull request may close these issues.
6 participants
@peng @sarah11918 @ematipico @bluwy @VoxelMC @Princesseuh