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

feat(core): better module not found handling #7079

Merged
merged 3 commits into from Mar 13, 2020
Merged

feat(core): better module not found handling #7079

merged 3 commits into from Mar 13, 2020

Conversation

clarkdo
Copy link
Member

@clarkdo clarkdo commented Mar 13, 2020

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

Fix nuxt/eslint-plugin-nuxt#87 fix nuxt/create-nuxt-app#423

If user installed dependecities with prod flag like yarn install --production or NODE_ENV=production yarn install, building (yarn nuxt build) will failed if buildModules exists in devDependecies.

This pr is checking the module existence and also print an info message for avoding unpected installation from user side.

@pi0 The info message is my own language, please feel free to make it better.

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.

@clarkdo clarkdo requested review from pi0 and a team March 13, 2020 12:21
@clarkdo clarkdo changed the title fix(core): ignore uninstalled build module in production build fix(core): ignore uninstalled build module in production installation Mar 13, 2020
@clarkdo
Copy link
Member Author

clarkdo commented Mar 13, 2020

Failed building seems related to actions/cache#208

@pi0
Copy link
Member

pi0 commented Mar 13, 2020

However, it is more a DX improvement, really cool idea @clarkdo 👍 I will update the message and also add a check for modules.

Update: Double checking issue, yarn nuxt build does require buildModules and devDependencies regardless of environment. Silently ignoring modules for production build can cause unwanted issues if the package is not installed.

The build phase requires devDependencies like @nuxt/builder. it is confusing naming of NPM. We were first using devModules to prevent this confusion.

I think we need to update programmatic usage to use new getNuxt/getBuilder exports that properly set _start flag to exclude buildModules for the production run. Meanwhile, we can add a patch to silently ignore only if not using standard CLI for the build via _cli flag check and also use a better message for DX.

Update:

Error when using nuxt build and missing buildModules:

image

Error for programmatic users:

image

@codecov-io
Copy link

Codecov Report

Merging #7079 into dev will increase coverage by 0.03%.
The diff coverage is 73.33%.

Impacted file tree graph

@@            Coverage Diff            @@
##             dev    #7079      +/-   ##
=========================================
+ Coverage   62.7%   62.74%   +0.03%     
=========================================
  Files         84       84              
  Lines       3376     3390      +14     
  Branches     920      924       +4     
=========================================
+ Hits        2117     2127      +10     
- Misses      1013     1016       +3     
- Partials     246      247       +1
Flag Coverage Δ
#unittests 62.74% <73.33%> (+0.03%) ⬆️
Impacted Files Coverage Δ
packages/core/src/module.js 94.25% <73.33%> (-4.38%) ⬇️

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 75a870d...b25cef0. Read the comment docs.

@pi0 pi0 changed the title fix(core): ignore uninstalled build module in production installation feat(core): better module not found handling Mar 13, 2020
@Austio
Copy link

Austio commented Jun 12, 2020

@pi0 would you all consider raising an exception in the event that this happens, could even be a configuration?

A good example,

  • you have a local module foo that you are developing and load that module properly
  • in the module, then require a file someNonexistingFIle that doesn't exist
  • you get an error that the foo module was not able to be found

@pi0
Copy link
Member

pi0 commented Jun 12, 2020

@Austio Error is thrown not found + not not found unless using programmatic mode. Would you please make a simple repo for your example?

@danielroe danielroe added the 2.x label Jan 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

why are dev dependencies needed at runtime ? Error: Cannot find module '@nuxtjs/eslint-module'
5 participants