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

Vite dev server crashes with appType mpa on root URI #10966

Closed
7 tasks done
wolfgangwalther opened this issue Nov 17, 2022 · 3 comments · Fixed by #10985
Closed
7 tasks done

Vite dev server crashes with appType mpa on root URI #10966

wolfgangwalther opened this issue Nov 17, 2022 · 3 comments · Fixed by #10985
Labels
contribution welcome p3-minor-bug An edge case that only affects very specific usage (priority)

Comments

@wolfgangwalther
Copy link

Describe the bug

Using appType: mpa without a <root>/index.html file will crash when requesting / instead of returning 404.

See #10336 (comment) for the change that introduced this.

Reproduction

https://stackblitz.com/edit/vitejs-vite-t9a2jr?file=vite.config.js

Steps to reproduce

Run the dev server and request /.

System Info

System:
    OS: Linux 5.15 Alpine Linux
    CPU: (32) x64 AMD Ryzen 9 3950X 16-Core Processor
    Memory: 104.02 GB / 125.75 GB
    Container: Yes
    Shell: 1.35.0 - /bin/ash
  Binaries:
    Node: 19.0.1 - /usr/local/bin/node
    Yarn: 3.2.4 - /usr/local/bin/yarn
    npm: 8.19.2 - /usr/local/bin/npm
  npmPackages:
    @vitejs/plugin-vue2: 2.0.1 => 2.0.1
    vite: 3.2.3 => 3.2.3

Used Package Manager

yarn

Logs

No response

Validations

@sun0day
Copy link
Member

sun0day commented Nov 18, 2022

For a quick solution, maybe we should allow mpa mode fallback to index.html as spa does.

@bluwy
Copy link
Member

bluwy commented Nov 18, 2022

Error:

21:36:21 [vite] Internal server error: Cannot read properties of undefined (reading 'charAt')

Looks like it's failing at:

	        if(rewriteTarget.charAt(0) !== '/') {
	          logger(
	            'We recommend using an absolute path for the rewrite target.',
	            'Received a non-absolute rewrite target',
	            rewriteTarget,
	            'for URL',
	            req.url
	          );
	        }

	        logger('Rewriting', req.method, req.url, 'to', rewriteTarget);
	        req.url = rewriteTarget;
	        return next();

We could return req.url here so it's a no-op to fix it:

if (spaFallback) {
return `/index.html`
}

@bluwy bluwy added contribution welcome p3-minor-bug An edge case that only affects very specific usage (priority) and removed pending triage labels Nov 18, 2022
@github-actions
Copy link

Hello @wolfgangwalther. We like your proposal/feedback and would appreciate a contribution via a Pull Request by you or another community member. We thank you in advance for your contribution and are looking forward to reviewing it!

sun0day pushed a commit to sun0day/vite that referenced this issue Nov 18, 2022
patak-dev pushed a commit that referenced this issue Nov 20, 2022
fc pushed a commit to fc/vite that referenced this issue Nov 23, 2022
bluwy pushed a commit that referenced this issue Nov 29, 2022
patak-dev pushed a commit that referenced this issue Nov 30, 2022
* fix(config): exclude config.assetsInclude empty array (#10941)

fixes #10926

* feat(css): upgrade postcss-modules (#10987)

* fix(mpa): support mpa fallback (#10985)

fixes #10966

* fix: export preprocessCSS in CJS (#11067)

* chore: update license

Co-authored-by: sun0day <sun_day500@163.com>
Co-authored-by: Jason Quense <monastic.panic@gmail.com>
Co-authored-by: sun0day <ivan.xu@applovin.com>
@github-actions github-actions bot locked and limited conversation to collaborators Dec 5, 2022
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
contribution welcome p3-minor-bug An edge case that only affects very specific usage (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants