Repository: nuxt/nuxt.js
Pull request #7518 (2020-06-15 11:53:27)

fix(generator): encode routes and show error summary
<!--- Provide a general summary of your changes in the title above -->

## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [x] Bug fix (a non-breaking change which fixes an issue)
- [ ] New feature (a non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)


## Description
<!--- Describe your changes in detail -->
<!--- Why is this change required? What problem does it solve? -->
<!--- If it resolves an open issue, please link to the issue here. For example "Resolves: #1337" -->
- Ensure generated routes are encoded (fs output is decoded) possibly fixing https://github.com/nuxt/nuxt.js/pull/6159#issuecomment-635013365 (reported by @rajesh-h)
- Fix DX issue by displaying an inline summary for handled errors

Before:
![image](https://user-images.githubusercontent.com/5158436/84654309-3b26f300-af0f-11ea-8409-6f805171040f.png)

After:
![image](https://user-images.githubusercontent.com/5158436/84654341-4aa63c00-af0f-11ea-9c2b-29a418f8b94b.png)


## Checklist:
<!--- Put an `x` in all the boxes that apply. -->
<!--- If your change requires a documentation PR, please link it appropriately -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly. (PR: #)
- [ ] I have added tests to cover my changes (if not applicable, please state why)
- [ ] All new and existing tests are passing.



rajesh-h: Hi Team,

I was unable to try this branch. Am I doing something wrong in my package.json?

Previous line -` "nuxt-edge": "^2.13.0-26509786.618eb5fa",`
I changed it as below
`"nuxt": "nuxt/nuxtjs#fix/static-encode",`

I get below error
```Output:
ERROR: Repository not found.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.```
codecov-commenter: # [Codecov](https://codecov.io/gh/nuxt/nuxt.js/pull/7518?src=pr&el=h1) Report
> Merging [#7518](https://codecov.io/gh/nuxt/nuxt.js/pull/7518?src=pr&el=desc) into [dev](https://codecov.io/gh/nuxt/nuxt.js/commit/ed34199146ee90e8dab23b08e3c2d621ada7bf2f&el=desc) will **increase** coverage by `0.00%`.
> The diff coverage is `66.66%`.

[![Impacted file tree graph](https://codecov.io/gh/nuxt/nuxt.js/pull/7518/graphs/tree.svg?width=650&height=150&src=pr&token=nGD1PtMB3M)](https://codecov.io/gh/nuxt/nuxt.js/pull/7518?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##              dev    #7518   +/-   ##
=======================================
  Coverage   70.18%   70.19%           
=======================================
  Files          88       88           
  Lines        3737     3738    +1     
  Branches     1017     1017           
=======================================
+ Hits         2623     2624    +1     
  Misses        905      905           
  Partials      209      209           
```

| Flag | Coverage Δ | |
|---|---|---|
| #unittests | `70.19% <66.66%> (+<0.01%)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/nuxt/nuxt.js/pull/7518?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [packages/generator/src/generator.js](https://codecov.io/gh/nuxt/nuxt.js/pull/7518/diff?src=pr&el=tree#diff-cGFja2FnZXMvZ2VuZXJhdG9yL3NyYy9nZW5lcmF0b3IuanM=) | `85.14% <66.66%> (+0.07%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/nuxt/nuxt.js/pull/7518?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/nuxt/nuxt.js/pull/7518?src=pr&el=footer). Last update [ed34199...11141d5](https://codecov.io/gh/nuxt/nuxt.js/pull/7518?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

pi0: @rajesh-h Linking monorepo packages should be one by one and needs a build step. I would suggest waiting to merge and test with `nuxt-edge` :) Changes almost done trying to fix tests.

Patch:
diff --git a/packages/generator/src/generator.js b/packages/generator/src/generator.js
index 6a67f6363..66c1b1338 100644
--- a/packages/generator/src/generator.js
+++ b/packages/generator/src/generator.js
@@ -23,7 +23,6 @@ export default class Generator {
       this.distPath,
       isUrl(this.options.build.publicPath) ? '' : this.options.build.publicPath
     )
-    this.generatedRoutes = new Set()
 
     // Shared payload
     this._payload = null
@@ -153,9 +152,16 @@ export default class Generator {
   async generateRoutes (routes) {
     const errors = []
 
-    this.routes = routes
-    // Add routes to the tracked generated routes (for crawler)
-    this.routes.forEach(({ route }) => this.generatedRoutes.add(route))
+    this.routes = []
+    this.generatedRoutes = new Set()
+
+    routes.forEach(({ route, ...props }) => {
+      route = decodeURI(route)
+      this.routes.push({ route, ...props })
+      // Add routes to the tracked generated routes (for crawler)
+      this.generatedRoutes.add(route)
+    })
+
     // Start generate process
     while (this.routes.length) {
       let n = 0
@@ -308,11 +314,11 @@ export default class Generator {
             .split('#')[0]
             .trim()
 
-          const href = sanitizedHref + possibleTrailingSlash
+          const route = decodeURI(sanitizedHref + possibleTrailingSlash)
 
-          if (href.startsWith('/') && !path.extname(href) && this.shouldGenerateRoute(href) && !this.generatedRoutes.has(href)) {
-            this.generatedRoutes.add(href) // add the route to the tracked list
-            this.routes.push({ route: href })
+          if (route.startsWith('/') && !path.extname(route) && this.shouldGenerateRoute(route) && !this.generatedRoutes.has(route)) {
+            this.generatedRoutes.add(route)
+            this.routes.push({ route })
           }
         })
       }
@@ -377,10 +383,10 @@ export default class Generator {
     await this.nuxt.callHook('export:routeCreated', { route, path: page.path, errors: pageErrors })
 
     if (pageErrors.length) {
-      consola.error('Error generating ' + route)
+      consola.error(`Error generating route "${route}": ${pageErrors.map(e => e.error.message).join(', ')}`)
       errors.push(...pageErrors)
     } else {
-      consola.success('Generated ' + route)
+      consola.success(`Generated route "${route}"`)
     }
 
     return true
diff --git a/packages/vue-app/template/App.js b/packages/vue-app/template/App.js
index 70d680a40..c05150ed5 100644
--- a/packages/vue-app/template/App.js
+++ b/packages/vue-app/template/App.js
@@ -311,7 +311,7 @@ export default {
       route = (route.replace(/\/+$/, '') || '/').split('?')[0]
       const src = urlJoin(base, staticAssetsBase, route, 'payload.js')
       try {
-        const payload = await window.__NUXT_IMPORT__(route, src)
+        const payload = await window.__NUXT_IMPORT__(decodeURI(route), encodeURI(src))
         this.setPagePayload(payload)
         return payload
       } catch (err) {


Test patch:
diff --git a/packages/generator/test/generator.gen.test.js b/packages/generator/test/generator.gen.test.js
index 951b7c86c..849adf3d7 100644
--- a/packages/generator/test/generator.gen.test.js
+++ b/packages/generator/test/generator.gen.test.js
@@ -95,12 +95,9 @@ describe('generator: generate routes', () => {
     const generator = new Generator(nuxt)
 
     generator.generateRoute = jest.fn()
-    jest.spyOn(routes, 'splice')
 
     const errors = await generator.generateRoutes(routes)
 
-    expect(routes.splice).toBeCalledTimes(3)
-    expect(routes.splice).toBeCalledWith(0, 2)
     expect(waitFor).toBeCalledTimes(5)
     expect(waitFor).nthCalledWith(1, 0)
     expect(waitFor).nthCalledWith(2, 100)
@@ -119,8 +116,6 @@ describe('generator: generate routes', () => {
 
     expect(generator._formatErrors).toBeCalledTimes(1)
     expect(generator._formatErrors).toBeCalledWith(errors)
-
-    routes.splice.mockRestore()
   })
 
   test('should format errors', () => {
diff --git a/packages/generator/test/generator.route.test.js b/packages/generator/test/generator.route.test.js
index 62f84083d..5c8512741 100644
--- a/packages/generator/test/generator.route.test.js
+++ b/packages/generator/test/generator.route.test.js
@@ -118,7 +118,7 @@ describe('generator: generate route', () => {
     const returned = await generator.generateRoute({ route, payload, errors })
 
     expect(consola.error).toBeCalledTimes(1)
-    expect(consola.error).toBeCalledWith('Error generating /foo')
+    expect(consola.error).toBeCalledWith('Error generating route "/foo": render route failed')
     expect(errors).toEqual([{
       error,
       route,
diff --git a/test/fixtures/full-static/layouts/default.vue b/test/fixtures/full-static/layouts/default.vue
index 264beabaf..fa9728073 100644
--- a/test/fixtures/full-static/layouts/default.vue
+++ b/test/fixtures/full-static/layouts/default.vue
@@ -10,8 +10,11 @@
       <NLink to="/pagination/1">
         Pagination
       </NLink>
+      <NLink to="/dynamic/foo bar">
+        Dynamic route 1
+      </NLink>
       <NLink to="/dynamic/foo%20bar">
-        Dynamic route
+        Dynamic route 2
       </NLink>
     </nav>
     <br>