Repository: nuxt/nuxt.js
Pull request #6490 (2019-10-01 08:26:03)

fix(core): nuxt resolver should clear require.cache in dev mode
<!--- 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" -->

Partial fix for #6232 (that if serverMiddleware is provided as string then on dev server restart the changes in the middleware are not picked up).

I think this also fixes the issue I have during module development which caused me to having to restart the dev server all the time because changed werent picked up.

Not sure how to add a test for this.

## 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.



codecov-io: # [Codecov](https://codecov.io/gh/nuxt/nuxt.js/pull/6490?src=pr&el=h1) Report
> Merging [#6490](https://codecov.io/gh/nuxt/nuxt.js/pull/6490?src=pr&el=desc) into [dev](https://codecov.io/gh/nuxt/nuxt.js/commit/37bdf7496e6591d7a9cc24d45080d4194ad87cc9?src=pr&el=desc) will **decrease** coverage by `45.09%`.
> The diff coverage is `80%`.

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

```diff
@@            Coverage Diff             @@
##              dev    #6490      +/-   ##
==========================================
- Coverage   95.66%   50.57%   -45.1%     
==========================================
  Files          79       79              
  Lines        2702     2705       +3     
  Branches      697      697              
==========================================
- Hits         2585     1368    -1217     
- Misses        101     1030     +929     
- Partials       16      307     +291
```

| Flag | Coverage Δ | |
|---|---|---|
| #e2e | `100% <ø> (ø)` | :arrow_up: |
| #fixtures | `50.57% <80%> (+0.01%)` | :arrow_up: |
| #unit | `?` | |

| [Impacted Files](https://codecov.io/gh/nuxt/nuxt.js/pull/6490?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [packages/core/src/resolver.js](https://codecov.io/gh/nuxt/nuxt.js/pull/6490/diff?src=pr&el=tree#diff-cGFja2FnZXMvY29yZS9zcmMvcmVzb2x2ZXIuanM=) | `79.71% <80%> (-20.29%)` | :arrow_down: |
| [packages/generator/src/generator.js](https://codecov.io/gh/nuxt/nuxt.js/pull/6490/diff?src=pr&el=tree#diff-cGFja2FnZXMvZ2VuZXJhdG9yL3NyYy9nZW5lcmF0b3IuanM=) | `0% <0%> (-100%)` | :arrow_down: |
| [packages/cli/src/index.js](https://codecov.io/gh/nuxt/nuxt.js/pull/6490/diff?src=pr&el=tree#diff-cGFja2FnZXMvY2xpL3NyYy9pbmRleC5qcw==) | `0% <0%> (-100%)` | :arrow_down: |
| [packages/cli/src/options/server.js](https://codecov.io/gh/nuxt/nuxt.js/pull/6490/diff?src=pr&el=tree#diff-cGFja2FnZXMvY2xpL3NyYy9vcHRpb25zL3NlcnZlci5qcw==) | `0% <0%> (-100%)` | :arrow_down: |
| [packages/cli/src/utils/banner.js](https://codecov.io/gh/nuxt/nuxt.js/pull/6490/diff?src=pr&el=tree#diff-cGFja2FnZXMvY2xpL3NyYy91dGlscy9iYW5uZXIuanM=) | `0% <0%> (-100%)` | :arrow_down: |
| [packages/cli/src/run.js](https://codecov.io/gh/nuxt/nuxt.js/pull/6490/diff?src=pr&el=tree#diff-cGFja2FnZXMvY2xpL3NyYy9ydW4uanM=) | `0% <0%> (-100%)` | :arrow_down: |
| [packages/cli/src/commands/start.js](https://codecov.io/gh/nuxt/nuxt.js/pull/6490/diff?src=pr&el=tree#diff-cGFja2FnZXMvY2xpL3NyYy9jb21tYW5kcy9zdGFydC5qcw==) | `0% <0%> (-100%)` | :arrow_down: |
| [packages/cli/src/options/common.js](https://codecov.io/gh/nuxt/nuxt.js/pull/6490/diff?src=pr&el=tree#diff-cGFja2FnZXMvY2xpL3NyYy9vcHRpb25zL2NvbW1vbi5qcw==) | `0% <0%> (-100%)` | :arrow_down: |
| [packages/cli/src/utils/constants.js](https://codecov.io/gh/nuxt/nuxt.js/pull/6490/diff?src=pr&el=tree#diff-cGFja2FnZXMvY2xpL3NyYy91dGlscy9jb25zdGFudHMuanM=) | `0% <0%> (-100%)` | :arrow_down: |
| [packages/cli/src/setup.js](https://codecov.io/gh/nuxt/nuxt.js/pull/6490/diff?src=pr&el=tree#diff-cGFja2FnZXMvY2xpL3NyYy9zZXR1cC5qcw==) | `0% <0%> (-100%)` | :arrow_down: |
| ... and [44 more](https://codecov.io/gh/nuxt/nuxt.js/pull/6490/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/nuxt/nuxt.js/pull/6490?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/6490?src=pr&el=footer). Last update [37bdf74...0d801fe](https://codecov.io/gh/nuxt/nuxt.js/pull/6490?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

kevinmarrec: @pi0 Any idea is it wouldn't cause issue to TypeScript runtime ?
pi0: @kevinmarrec Good point. We can try to ensure. I remember there was something wrong. But can try :)
kevinmarrec: Yeah I don't remember well what was going on around `ts-node` require cache.. It may be not an issue anymore 🤷‍♂️

Patch:
diff --git a/packages/core/src/resolver.js b/packages/core/src/resolver.js
index 52803afe8..bcded60ef 100644
--- a/packages/core/src/resolver.js
+++ b/packages/core/src/resolver.js
@@ -3,7 +3,12 @@ import fs from 'fs-extra'
 import consola from 'consola'
 import esm from 'esm'
 
-import { startsWithRootAlias, startsWithSrcAlias } from '@nuxt/utils'
+import {
+  startsWithRootAlias,
+  startsWithSrcAlias,
+  isExternalDependency,
+  clearRequireCache
+} from '@nuxt/utils'
 
 export default class Resolver {
   constructor (nuxt) {
@@ -138,9 +143,17 @@ export default class Resolver {
       lastError = e
     }
 
+    const isExternal = isExternalDependency(resolvedPath)
+
+    // in dev mode make sure to clear the require cache so after
+    // a dev server restart any changed file is reloaded
+    if (this.options.dev && !isExternal) {
+      clearRequireCache(resolvedPath)
+    }
+
     // By default use esm only for js,mjs files outside of node_modules
     if (useESM === undefined) {
-      useESM = /.(js|mjs)$/.test(resolvedPath) && !/node_modules/.test(resolvedPath)
+      useESM = !isExternal && /.(js|mjs)$/.test(resolvedPath)
     }
 
     // Try to require
diff --git a/packages/utils/src/cjs.js b/packages/utils/src/cjs.js
index 453fca7da..8b06c4350 100644
--- a/packages/utils/src/cjs.js
+++ b/packages/utils/src/cjs.js
@@ -1,6 +1,10 @@
+export function isExternalDependency (id) {
+  return /[/\\]node_modules[/\\]/.test(id)
+}
+
 export function clearRequireCache (id) {
   const entry = require.cache[id]
-  if (!entry || id.includes('node_modules')) {
+  if (!entry || isExternalDependency(id)) {
     return
   }
 
@@ -17,7 +21,7 @@ export function clearRequireCache (id) {
 
 export function scanRequireTree (id, files = new Set()) {
   const entry = require.cache[id]
-  if (!entry || id.includes('node_modules') || files.has(id)) {
+  if (!entry || isExternalDependency(id) || files.has(id)) {
     return files
   }
 


Test patch:
diff --git a/packages/core/test/resolver.test.js b/packages/core/test/resolver.test.js
index 8c3ac46e5..4b0deb448 100644
--- a/packages/core/test/resolver.test.js
+++ b/packages/core/test/resolver.test.js
@@ -442,7 +442,7 @@ describe('core: resolver', () => {
       const resolver = new Resolver({
         options: {}
       })
-      resolver.resolvePath = jest.fn()
+      resolver.resolvePath = jest.fn().mockReturnValue('/var/nuxt/resolver/file.js')
       resolver.esm = jest.fn()
 
       resolver.requireModule('/var/nuxt/resolver/file.js', { esm: true })
diff --git a/packages/utils/test/cjs.test.js b/packages/utils/test/cjs.test.js
new file mode 100644
index 000000000..4b5c1b977
--- /dev/null
+++ b/packages/utils/test/cjs.test.js
@@ -0,0 +1,14 @@
+import { isExternalDependency, getPKG } from '../src/cjs'
+
+describe('util: cjs', () => {
+  test('isExternalDependency works', () => {
+    expect(isExternalDependency('/var/nuxt/node_modules/dependency.js')).toBe(true)
+    expect(isExternalDependency('/var/nuxt/dependency.js')).toBe(false)
+    expect(isExternalDependency('C:\\nuxt\\node_modules\\dependency.js')).toBe(true)
+    expect(isExternalDependency('C:\\nuxt\\dependency.js')).toBe(false)
+  })
+
+  test('getPKG should not throw error on non existing package', () => {
+    expect(() => getPKG('this-doesnt-existsw')).not.toThrow()
+  })
+})