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: validate sources value to be string #32

Merged
merged 3 commits into from
Sep 5, 2022
Merged

fix: validate sources value to be string #32

merged 3 commits into from
Sep 5, 2022

Conversation

danielroe
Copy link
Member

Currently using a package that injects an object as theme in nuxt.config and is broken since nuxt/framework#7131. Obviously that will need to be fixed in the theme, but in a similar case in future it will be unclear what the issue is because it triggers the following error:

Nuxi 3.0.0-rc.10-27706138.b31405f
Nuxt 3.0.0-rc.10-27706138.b31405f with Nitro 0.5.1-27706249.08ac81b

 ERROR  Cannot start nuxt:  source.startsWith is not a function

  at node_modules/.pnpm/c12@0.2.10/node_modules/c12/dist/index.mjs:174:44
  at Array.some (<anonymous>)
  at resolveConfig (node_modules/.pnpm/c12@0.2.10/node_modules/c12/dist/index.mjs:174:20)
  at extendConfig (node_modules/.pnpm/c12@0.2.10/node_modules/c12/dist/index.mjs:151:27)
  at extendConfig (node_modules/.pnpm/c12@0.2.10/node_modules/c12/dist/index.mjs:156:11)
  at async loadConfig (node_modules/.pnpm/c12@0.2.10/node_modules/c12/dist/index.mjs:114:5)

This change makes it warn instead:

 WARN  Cannot extend config from {"meta":{"name":"Nuxt Elements","description":"Building blocks for your next application.","author":"NuxtLabs"}} in /private/tmp/studio/node_modules/.pnpm/@nuxt-themes+elements-edge@0.0.1-fdfc0b1/node_modules/@nuxt-themes/elements-edge/theme

It would also be equally easy to filter it out at L136 but thought you might prefer the warning for now.

@danielroe danielroe requested a review from pi0 September 5, 2022 14:03
@danielroe danielroe self-assigned this Sep 5, 2022
src/loader.ts Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Sep 5, 2022

Codecov Report

Merging #32 (7ee2658) into main (faa482d) will decrease coverage by 1.31%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main      #32      +/-   ##
==========================================
- Coverage   76.38%   75.07%   -1.32%     
==========================================
  Files           3        3              
  Lines         343      349       +6     
  Branches       46       47       +1     
==========================================
  Hits          262      262              
- Misses         36       41       +5     
- Partials       45       46       +1     
Impacted Files Coverage Δ
src/loader.ts 77.57% <0.00%> (-2.24%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@pi0 pi0 changed the title fix: check source value in case it isn't string fix: validate sources value to be string Sep 5, 2022
@pi0 pi0 merged commit f97c850 into main Sep 5, 2022
@pi0 pi0 deleted the fix/check-source branch September 5, 2022 18:05
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