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: prevent options merging fn exec without default value #287

Merged
merged 2 commits into from
Jul 27, 2020

Conversation

benjamincanac
Copy link
Member

Functions in configuration are meant to allow merging default values. Now, functions are only executed if the value already exists in the configuration.

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

When using extendParser with a function '.txt': (file) => ({ body: file }), the function was executed.

@mathe42
Copy link
Contributor

mathe42 commented Jul 23, 2020

So if we want to overwrite an existing parser we have to use:

{
  '.xml': file => parse(file)
}

For a new extension we can use:

{
  '.custom': file => parse(file)
}

~

@codecov-commenter
Copy link

codecov-commenter commented Jul 23, 2020

Codecov Report

❗ No coverage uploaded for pull request base (dev@5ffd451). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##             dev     #287   +/-   ##
======================================
  Coverage       ?   90.21%           
======================================
  Files          ?       15           
  Lines          ?      552           
  Branches       ?      118           
======================================
  Hits           ?      498           
  Misses         ?       48           
  Partials       ?        6           

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 5ffd451...5b836ad. Read the comment docs.

@mathe42
Copy link
Contributor

mathe42 commented Jul 23, 2020

Maybe we can add one sentence in the docs for this...

@mathe42
Copy link
Contributor

mathe42 commented Jul 23, 2020

Never mind I was wrong...

Copy link
Contributor

@mathe42 mathe42 left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@benjamincanac benjamincanac merged commit 8e5318a into dev Jul 27, 2020
@benjamincanac benjamincanac deleted the fix/options-merging-defaults branch July 27, 2020 08:21
benjamincanac added a commit that referenced this pull request Jul 27, 2020
* fix(lib): prevent options merging fn exec without default value

* test: fix extend parser options test
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.

4 participants