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 enhance & append conflict check #3216

Merged
merged 2 commits into from
Mar 19, 2018

Conversation

monoblaine
Copy link
Contributor

Description:

Preface: We should not use enhance: true and append: true at the same time, so Ractive checks for this case and throws an error if we misbehave.

However, if we use Ractive.defaults.enhance = true; and create a Ractive instance with append: true, Ractive fails to throw an error.

That's because here

// disallow combination of `append` and `enhance`
if (options.append && options.enhance) {
  throw new Error('Cannot use append and enhance at the same time');
}

we're using the options object to check for the conflicting case. I guess we should use target instead of options to do this.

Is breaking:

It was breaking the Cannot use append and enhance at the same time test in enhance.js because options.enhance was not getting copied into target. And that's because the defaults object does not include enhance. That's why I added that in 4bdefbd.

@evs-chris evs-chris merged commit 40704fe into ractivejs:dev Mar 19, 2018
@evs-chris
Copy link
Contributor

👍 Thanks!

Typically I'd ask for a test as well, but this is a very simple change and tests with global state are icky 😄

@monoblaine monoblaine deleted the options-and-defaults branch March 19, 2018 21:28
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.

None yet

2 participants