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(builder): corretly detect mode of hashed plugins #5695

Merged
merged 1 commit into from
May 11, 2019
Merged

Conversation

pi0
Copy link
Member

@pi0 pi0 commented May 10, 2019

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

moduleContainer.addPlugin generates a hash suffixed name if no explicit fileName is provided to prevent name collisions inside .nuxt. This PR ensures mode of file names like mode-test.plugin.client.530b6c6a.js is also correctly detected.

fixes #5694

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.

@pi0 pi0 requested a review from clarkdo May 10, 2019 19:40
@codecov-io
Copy link

codecov-io commented May 10, 2019

Codecov Report

Merging #5695 into dev will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##             dev   #5695   +/-   ##
=====================================
  Coverage   95.6%   95.6%           
=====================================
  Files         82      82           
  Lines       2664    2664           
  Branches     683     683           
=====================================
  Hits        2547    2547           
  Misses        98      98           
  Partials      19      19
Impacted Files Coverage Δ
packages/builder/src/builder.js 100% <100%> (ø) ⬆️

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 545afa8...3b2e445. Read the comment docs.

Copy link
Member

@clarkdo clarkdo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any file’s name includes client or server will be considered as mode plugin, like foo.client.bar.hash.js, is this behavior appropriate?

How about limting 0-2 times on .\w or explicitly setting mode in addPlugin before hash generation

@pi0
Copy link
Member Author

pi0 commented May 10, 2019

@clarkdo As the pattern is like .(client|server|all). I think it is safe to just allow extending suffix. Resolving mode in addPlugin introduces duplicate code. Any concerns for not allowing more than 2 suffixes?

@clarkdo
Copy link
Member

clarkdo commented May 10, 2019

Maybe an inappropriate example: it may be a potential breaking change for sth like foo.client.bar.js or foo.client.server.js

@clarkdo
Copy link
Member

clarkdo commented May 10, 2019

In normal usage, it’s fine, so feel free to merge.

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.

Plugin Filename drives mode via addPlugin
4 participants