[Fix #17612][V4] Throw explicit error when a nonexistent method is invoked

[Johann-S]

Hi

it's a fix for #17612

[Johann-S]

I changed this line because template string not working with QUnit test, so all my test cases failed

[cvrebert]

Leave this at 2. I'll figure out how to exempt the tests from this particular ESLint rule.

[Johann-S]

I think we can use this in js files : /*eslint prefer-template: 0*/ so it would be ok
What do you think about this ?

[cvrebert]

Sounds good

[Johann-S]

Travis dislike my commit 😢

[cvrebert]

GitHub isn't showing it for some reason, but looks like the forced rebuild succeeded: https://travis-ci.org/twbs/bootstrap/jobs/80598464

[Johann-S]

Oh that's it thank you

[cvrebert]

@Johann-S What failure did you encounter when you used template strings?

[Johann-S]

When I ran my test case the template wasn't working.

In my JS src I wrote :
throw new Error('No method named "${config}"')

And in my test case I check
assert.strictEqual(err.message, 'No method named "noMethod"')

But err.message was equal to : No method named "${config}"
so the ${config} wasn't replaced by the value

for exemple :

[cvrebert]

In my JS src I wrote :
throw new Error('No method named "${config}"')

Template strings are enclosed in backticks, not single quotes.
So it should be:
throw new Error(No method named "${config}")

[Johann-S]

Thank you very much ! I wasn't aware of that.

I made a few changes : removed /*eslint prefer-template: 0*/ and using backsticks

build passed : https://travis-ci.org/twbs/bootstrap/builds/80774970

[cvrebert]

Why'd you change the condition here?

[Johann-S]

because action is always a string.

But if you prefer I can restore the old condition

[cvrebert]

I guess it's okay.

[cvrebert]

Thanks for all the revising. I'll be happy to merge this once the undefined change is made.

[Johann-S]

Thank you for your review and feedbacks you helped me a lot ! 👍

[cvrebert]

Use method here instead of data[config]

[Johann-S]

The problem is when I replace data[config] by method a lot of use cases failed.

For exemple if I change this in Tooltip file I have 43 tests failed with the same error message :
TypeError: Cannot read property 'constructor' of undefined

Because the this isn't a Tooltip object when I call a method with method() so this why I let
data[config]() because data is a Tooltip object.

Any thoughts about this @cvrebert ?
Maybe it's better to do this :

if (data[config] === undefined) {
    throw new Error(`No method named "${config}"`)
}
data[config]()

[cvrebert]

Ah, right, JS doesn't hold onto this...
There wasn't much difference in the microbenchmark I tried (http://jsperf.com/call-vs-reget), so..

Maybe it's better to do this :

@Johann-S Yeah, do that.

[Johann-S]

Done !
I removed all let method because it's not relevant for just a condition and a method call.

[cvrebert]

Thank you for your patience!

[Johann-S]

You're welcome and thank you for your help and feedbacks 👍