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(utils): handle serializeFunction edge case #5754

Merged
merged 1 commit into from
May 19, 2019
Merged

Conversation

aaronransley
Copy link
Contributor

@aaronransley aaronransley commented May 16, 2019

Resolves #5001 and cases where functions are serialized with a prefix of function function

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

This is a naive attempt at resolving #5001

I am not certain how to resolve the root issue with the function parsing code; the current code in serialize.js strikes me as requiring specific knowledge of how it was built. @galvez Perhaps you have ideas for a better fix?

Otherwise, I'm hopeful that this PR is helpful!

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.

Resolves edge cases where functions are serialized with a prefix of `function function`
@aaronransley aaronransley changed the title Monkeypatch to serializeFunction fix(utils) Monkeypatch to serializeFunction May 16, 2019
@codecov-io
Copy link

codecov-io commented May 16, 2019

Codecov Report

Merging #5754 into dev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##             dev   #5754   +/-   ##
=====================================
  Coverage   95.6%   95.6%           
=====================================
  Files         82      82           
  Lines       2662    2662           
  Branches     683     683           
=====================================
  Hits        2545    2545           
  Misses        98      98           
  Partials      19      19
Flag Coverage Δ
#e2e ?
#fixtures ?
#unit ?
Impacted Files Coverage Δ
packages/utils/src/serialize.js 92.85% <ø> (ø) ⬆️

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 b0f9552...e6db65d. Read the comment docs.

@pi0 pi0 requested a review from galvez May 16, 2019 21:53
Copy link

@galvez galvez left a comment

Choose a reason for hiding this comment

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

LGTM @pi0

@galvez galvez changed the title fix(utils) Monkeypatch to serializeFunction fix(utils): handle serializeFunction edge case May 16, 2019
@galvez
Copy link

galvez commented May 16, 2019

@aaronransley just please check this against 2.7.1 as I have a suspicion this may have been solved already. cc @pi0

@aaronransley
Copy link
Contributor Author

aaronransley commented May 16, 2019

@galvez Still occurring on 2.7.1.

I have updated the original issue #5001 to reflect this. CodeSanbox is reproducing

@galvez
Copy link

galvez commented May 16, 2019

@aaronransley k, cool, let's get this in then

@pi0 I'll take a stab at rewriting serializeFunction using acorn to prevent further problems.

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.

Syntax errors when supplying external function to scrollBehavior key in nuxt.config.js router config
5 participants