Display proper error when specified plugin isn't found #3672
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #3672 +/- ##
==========================================
- Coverage 98.94% 98.86% -0.09%
==========================================
Files 18 18
Lines 1134 1142 +8
Branches 304 307 +3
==========================================
+ Hits 1122 1129 +7
- Misses 12 13 +1
Continue to review full report at Codecov.
|
|
Have you looked into the comment from pi0 in #3434? 🤔
|
|
@manniL Yep, you'll notice the original PR doesn't apply r() to the paths.
|
There was a problem hiding this comment.
Where is better to add this check ?
- https://github.com/nuxt/nuxt.js/blob/cb18aa6f534137f4a8c462404e9b39850a13391f/lib/builder/builder.js#L65
- Inside a same loop like previous pr
- Current pr
| @@ -43,4 +43,16 @@ describe('nuxt', () => { | |||
|
|
|||
| await nuxt.close() | |||
| }) | |||
|
|
|||
| test('Fail to build when specified plugin isn\'t found', () => { | |||
| const nuxt = new Nuxt({ | |||
There was a problem hiding this comment.
Run a complete build here seems too expensive
There was a problem hiding this comment.
@clarkdo Not so much because it very quickly fails and exits.
|
@clarkdo good call re: |
lib/builder/builder.js
Outdated
| if (!exists) { | ||
| consola.warn(`Plugin not found: ${plugin.src}`) | ||
| } | ||
| return exists |
There was a problem hiding this comment.
Can’t return Boolean here, need plugin
|
Sorry, I should have let you known I wasn't done working on this.
|
|
@clarkdo I changed my approach entirely now. Can you read the PR again? |
lib/builder/builder.js
Outdated
| }).map((p) => ({ | ||
| src: this.nuxt.resolveAlias(p.src), | ||
| ssr: p.ssr !== false, | ||
| name: 'nuxt_plugin_' + p.basename + '_' + hash(p.src) |
There was a problem hiding this comment.
Any reason for moving base name calculation above?
There was a problem hiding this comment.
@clarkdo so we don't do it twice. I need that same calculation to display any potential warnings in the preceding filter().
lib/builder/builder.js
Outdated
| }).filter((p) => { | ||
| exists = fs.existsSync(this.nuxt.resolveAlias(p.src)) | ||
| if (!exists) { | ||
| consola.warn(`Plugin not found: ${p.basename}`) |
There was a problem hiding this comment.
I think you can use reduce instead of filter+map.
And I didn’t see any pleace else in this method is using basename, so it should be unnecessary to extract it calculation
lib/builder/builder.js
Outdated
| } | ||
| const basename = path.basename(p.src, path.extname(p.src)) | ||
| .replace(/[^a-zA-Z?\d\s:]/g, '') | ||
| return [ ...arr, { |
There was a problem hiding this comment.
Here everytime will create a new array, how about：
arr.push({
src: pluginPath,
ssr: p.ssr !== false,
name: 'nuxt_plugin_' + basename + '_' + hash(p.src)
})
return arr
lib/builder/builder.js
Outdated
| } | ||
| const basename = path.basename(p.src, path.extname(p.src)) | ||
| .replace(/[^a-zA-Z?\d\s:]/g, '') | ||
| return [...arr, { |
There was a problem hiding this comment.
Here everytime will create a new array, how about：
arr.push({
src: pluginPath,
ssr: p.ssr !== false,
name: 'nuxt_plugin_' + basename + '_' + hash(p.src)
})
return arr
ghost
mentioned this pull request
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Picking up on #3434 -- synced with dev and tests included.