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: ensure langDir is escaped on Windows #1097

Merged
merged 2 commits into from
Mar 8, 2021
Merged

Conversation

kedrzu
Copy link

@kedrzu kedrzu commented Mar 8, 2021

After updating nuxt-i18n my project stopped compiling.
After some digging, I've found this problem:
image

I'm working on windows, and paths do use backslashes.
When creating script templates this requires for string to be properly escaped.

It's enough to pass it through JSON.stringify, so I think easiest way to fix that, was to remove the special case for string.
Also, checking for boolean and null should also be unnecessary - JSON.stringify should handle it as well.

PS: this would also break if there were ' characters anywhere inside.

@rchl
Copy link
Collaborator

rchl commented Mar 8, 2021

I think the issue is deeper than that. Do the ASYNC_LOCALES variables look OK in the options.js?

I think it will look something like this which doesn't seem correct:
Screenshot 2021-03-08 at 14 19 26

@rchl
Copy link
Collaborator

rchl commented Mar 8, 2021

Or maybe that's just on my system because I'm forcing a Windows path on MacOS?

@kedrzu
Copy link
Author

kedrzu commented Mar 8, 2021

It didn't look so bad. Slashes were mixed (backslashes and standard ones), but they are at least escaped properly.

src/templates/options.js Outdated Show resolved Hide resolved
@rchl rchl changed the title fix: langDir path must be escaped on windows fix: ensure langDir is escaped on Windows Mar 8, 2021
@rchl rchl merged commit a0a3adc into nuxt-modules:master Mar 8, 2021
@rchl
Copy link
Collaborator

rchl commented Mar 8, 2021

Thanks!
Released in v6.20.6

This was referenced Mar 9, 2021
This was referenced Mar 15, 2021
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.

2 participants