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

feat: fix the use of vite.base in Astro Dev Server #13003

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

chaegumi
Copy link
Contributor

@chaegumi chaegumi commented Jan 18, 2025

Changes

Resolve this issue

#7520

The problem solved by this PR is that after setting the base, when pnpm run dev, the src address of the generated script type="module" tag is incorrect,

export default defineConfig({
  base: '/theme141',
})
<script type="module" src="/@vite/client"></script>
<script type="module" src="/@id/astro:scripts/page.js"></script>
<script type="module" src="/node_modules/"></script>

It still starts with/node_modules.

The expectation should start with the value of base,

<script type="module" src="/theme141/@vite/client"></script>
<script type="module" src="/theme141/@id/astro:scripts/page.js"></script>
<script type="module" src="/theme141/node_modules/"></script>

I found that modifying the base of the root node of astro.config.mjs does not work. I need to set vite.base so that it can be parsed correctly.

export default defineConfig({
  // base: '/ theme141',
  vite: {
    base: '/theme141'
  }
})

For example:

When using 127.0.0.1 or localhost, the generated resources can be accessed normally even if the address is incorrect.

But when I resolve a domain name locally

C:\Windows\System32\drivers\etc\hosts
127.0.0.1 example.local

Then when I use example.local/theme141 to access it, there will be a path error. Not setting the root base, only setting vite.base, and then cooperating with my modifications, can make the program run perfectly

Testing

Docs

Copy link

changeset-bot bot commented Jan 18, 2025

🦋 Changeset detected

Latest commit: 0930a6c

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) docs pr labels Jan 18, 2025
@chaegumi
Copy link
Contributor Author

#7520 (comment)

Copy link

codspeed-hq bot commented Jan 18, 2025

CodSpeed Performance Report

Merging #13003 will not alter performance

Comparing chaegumi:nginx-proxy_pass-dev-mode (0930a6c) with main (f576519)

Summary

✅ 6 untouched benchmarks

@github-actions github-actions bot added the semver: major Change triggers a `major` release label Jan 19, 2025
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This PR is blocked because it contains a major changeset. A reviewer will merge this at the next release if approved.

Copy link
Contributor

@ascorbic ascorbic 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 your contribution. I've left a few comments, but before you do more we need a much more detailed description in the PR of what exactly it is trying to do. need more info to see if this is something that needs doing. If this is just something for nginx then it's not something we're likely to add - it needs to show what problem it is solving, and how. It will also need tests, proabably ones that fail before your PR is added. I

@@ -241,6 +241,7 @@ type AstroContainerConstructor = {
streaming?: boolean;
renderers?: SSRLoadedRenderer[];
manifest?: AstroContainerManifest;
config?: any;
Copy link
Contributor

Choose a reason for hiding this comment

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

You already have this as viteConfig below. We also can't have any

'astro': patch
---

nginx proxy pass dev mode
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs a full description of what this PR does, phrased as if it's prefixed with "This PR..."

@ascorbic ascorbic removed the semver: major Change triggers a `major` release label Jan 20, 2025
@chaegumi chaegumi closed this Jan 21, 2025
@chaegumi chaegumi reopened this Jan 21, 2025
@chaegumi
Copy link
Contributor Author

chaegumi commented Jan 21, 2025

The problem solved by this PR is that after setting the base, when pnpm run dev, the src address of the generated script type="module" tag is incorrect,

export default defineConfig({
  base: '/theme141',
})
<script type="module" src="/@vite/client"></script>
<script type="module" src="/@id/astro:scripts/page.js"></script>
<script type="module" src="/node_modules/"></script>

It still starts with/node_modules.

The expectation should start with the value of base,

<script type="module" src="/theme141/@vite/client"></script>
<script type="module" src="/theme141/@id/astro:scripts/page.js"></script>
<script type="module" src="/theme141/node_modules/"></script>

I found that modifying the base of the root node of astro.config.mjs does not work. I need to set vite.base so that it can be parsed correctly.

export default defineConfig({
  // base: '/ theme141',
  vite: {
    base: '/theme141'
  }
})

For example:

When using 127.0.0.1 or localhost, the generated resources can be accessed normally even if the address is incorrect.

But when I resolve a domain name locally

C:\Windows\System32\drivers\etc\hosts
127.0.0.1 example.local

Then when I use example.local/theme141 to access it, there will be a path error. Not setting the root base, only setting vite.base, and then cooperating with my modifications, can make the program run perfectly

@ematipico
Copy link
Member

Thank you @chaegumi for your comment. Please update the title and description of the PR to better reflect what you're trying to solve. Also, add some tests

@chaegumi chaegumi changed the title Nginx proxy pass dev mode feat: fix the use of vite.base in Astro Dev Server Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs pr pkg: astro Related to the core `astro` package (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants