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

🐛 BUG: Building a site using React components with escaped spaces in path fails #3639

Closed
1 task
jackmerrill opened this issue Jun 18, 2022 · 6 comments · Fixed by #4584
Closed
1 task
Assignees
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority)

Comments

@jackmerrill
Copy link
Contributor

What version of astro are you using?

1.0.0-beta.47

Are you using an SSR adapter? If so, which one?

Node

What package manager are you using?

yarn

What operating system are you using?

Windows

Describe the Bug

When building an Astro site using the Node SSR adapter, as well with a React component placed in a folder with a space in it's name will fail.

Vite is unable to load the component during build time, thus failing the build.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-mhnkxf

Participation

  • I am willing to submit a pull request for this issue.
@bholmesdev bholmesdev added the - P2: has workaround Bug, but has workaround (priority) label Jun 20, 2022
@bholmesdev
Copy link
Contributor

Good catch @jackmerrill! Noting a few more details here:

  • fails on production builds without an SSR adapter as well
  • occurs on MacOS too
  • does not occur in plain Vite projects or similarly SSR'd projects like SvelteKit

@FredKSchott FredKSchott added - P3: minor bug An edge case that only affects very specific usage (priority) and removed - P2: has workaround Bug, but has workaround (priority) labels Jul 13, 2022
@FredKSchott
Copy link
Member

Another user hit this in Discord: https://discord.com/channels/830184174198718474/845451724738265138/996131165553557625

Bumping up the priority

@FredKSchott
Copy link
Member

We should have a test for a folder with a space in it, using a framework like React! @natemoo-re thinks we might already, but it probably isn't testing the framework part of this bug.

@Princesseuh
Copy link
Member

I investigated this a bit with my limited rollup, vite knowledge and here's where I got:

Our compiler's transform returns the modules paths encoded, I first thought this was the issue, but making it return the proper URLs didn't fix the issue. It seems like due to something in between the compiler and the build process, the URLs get encoded back. On this line rid should be the actual non-encoded URLs, but it is not.

Manually adding a decodeURI there fix the issue, as this commit shows. Another decodeURI is also needed in core/build/generate.ts as the path is also encoded there (might be using the same initial source? Not sure)

I spent a bit too much time on this, and I fear that the issue might lie in some pieces of code that we don't control / I don't know

@Princesseuh Princesseuh removed their assignment Aug 23, 2022
@FredKSchott FredKSchott added - P4: important Violate documented behavior or significantly impacts performance (priority) and removed - P3: minor bug An edge case that only affects very specific usage (priority) labels Aug 28, 2022
@FredKSchott
Copy link
Member

Thanks @Princesseuh!! bumping up the priority again given the recent reports. Also, assigning @bluwy who may have a bit more familiarity with this part of the codebase (cc @matthewp as well!)

@bluwy
Copy link
Member

bluwy commented Aug 29, 2022

I looked into this today and came with a similar fix at 04e93d1, which fixes the build issue with the repro.

#4263 have also added tests, but I also found that other symbols like 🚀 and % is making dev/build fail too. It seems to be some sourcemaps issue but I haven't dug too deep yet as it had also spent quite some time on this now. 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants