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(webpack): reduce false negatives in nodeExternals (#7462) #7464

Merged
merged 3 commits into from
Jun 9, 2020
Merged

fix(webpack): reduce false negatives in nodeExternals (#7462) #7464

merged 3 commits into from
Jun 9, 2020

Conversation

JohannesLamberts
Copy link

@JohannesLamberts JohannesLamberts commented Jun 4, 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)

This could change behaviour in existing SSR-Apps. The behaviour is however undocumented and unintended (afaik).

Description

Resolves: #7462

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.

I'm not sure how/if the changes can be tested and would be grateful for guiding/support.

@JohannesLamberts JohannesLamberts changed the title fix(webpack): reduce false negatives in nodeExternals (#7462) fix(webpack): reduce false negatives in nodeExternals Jun 4, 2020
@JohannesLamberts JohannesLamberts changed the title fix(webpack): reduce false negatives in nodeExternals fix(webpack): reduce false negatives in nodeExternals (#7462) Jun 4, 2020
@codecov-commenter
Copy link

codecov-commenter commented Jun 4, 2020

Codecov Report

Merging #7464 into dev will decrease coverage by 0.04%.
The diff coverage is 25.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #7464      +/-   ##
==========================================
- Coverage   70.32%   70.28%   -0.05%     
==========================================
  Files          88       88              
  Lines        3704     3708       +4     
  Branches     1010     1011       +1     
==========================================
+ Hits         2605     2606       +1     
- Misses        892      894       +2     
- Partials      207      208       +1     
Flag Coverage Δ
#unittests 70.28% <25.00%> (-0.05%) ⬇️
Impacted Files Coverage Δ
packages/webpack/src/config/server.js 2.38% <25.00%> (+2.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 cbbf665...4354c29. Read the comment docs.

@JohannesLamberts
Copy link
Author

@pi0 is there anything more to do for me or can this be merged?

@pi0
Copy link
Member

pi0 commented Jun 8, 2020

@JohannesLamberts No, actually it is perfect. Would be nice removing jsx as well if you have time :)

@JohannesLamberts
Copy link
Author

@pi0 done, I also removed the RegEx to reduce complexity

@pi0
Copy link
Member

pi0 commented Jun 8, 2020

Amazing thanks ❤️ Waiting for tests to pass

@JohannesLamberts
Copy link
Author

@pi0 https://github.com/nuxt/nuxt.js/pull/7464/checks?check_run_id=750834024 failed with

jest packages --forceExit --coverage
/bin/sh: 1: jest: not found

I cannot see how this could be related to the code changes :/

@pi0 pi0 merged commit 846cfee into nuxt:dev Jun 9, 2020
@pi0 pi0 mentioned this pull request Jun 10, 2020
@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
4 participants