Throw an error when layout property is not string

[suusan2go]

Types of changes

• 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

Resolves: #4035

Avoid using non-string value as layout property.

Checklist:

• 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 Report

Merging #4142 into dev will increase coverage by 0.44%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             dev    #4142      +/-   ##
=========================================
+ Coverage   81.8%   82.24%   +0.44%     
=========================================
  Files         29       29              
  Lines       1561     1549      -12     
  Branches     409      408       -1     
=========================================
- Hits        1277     1274       -3     
+ Misses       224      216       -8     
+ Partials      60       59       -1

Impacted Files	Coverage Δ
packages/core/src/nuxt.js	95.74% <0%> (-0.09%)	⬇️
packages/common/src/utils.js	80.72% <0%> (ø)	⬆️
packages/cli/src/common/utils.js	0% <0%> (ø)	⬆️
packages/cli/src/commands/dev.js	0% <0%> (ø)	⬆️
packages/builder/src/webpack/base.js	98.41% <0%> (ø)	⬆️
packages/common/src/options.js	86.23% <0%> (ø)	⬆️
packages/builder/src/builder.js	96.46% <0%> (ø)	⬆️
packages/builder/src/webpack/utils/postcss.js	95.34% <0%> (ø)	⬆️
packages/core/src/renderer.js	100% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b0074ce...7c9ffca. Read the comment docs.

[pi0]

Am I missing something or we should support layout as a function for dynamic layouts?

[manniL]

@pi0 Dang, you are correct! Almost forget that use case 🙈

@suusan2go Could you do a check for string or function? Sorry!

[suusan2go]

@pi0 @manniL
I think check for function is unnecessary.
because if layout is a function, it seems to call layout = layout(app.context) before setLayout.
https://github.com/nuxt/nuxt.js/blob/dev/packages/app/template/server.js#L123-L128
https://github.com/nuxt/nuxt.js/blob/dev/packages/app/template/client.js#L455-L464

Although I checked the examples/dynamic-layouts and it works perfect.

[pi0]

@suusan2go. Thanks for checking that. I just wanted to to endure that it's happening.

[manniL]

Well, I fast to fast with my conclusion there 🙈 @suusan2go Thank you for looking into it!

The last thing I'd consider is maybe only throwing an Error if you are in debug mode 🤔

[suusan2go]

@manniL Thank you for your comment! I changed my code to throw an error only in debug mode.

[lock]

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.