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 inconsistent has*Target property names in different locales #93

merged 1 commit into from Feb 2, 2018


Copy link

@javan javan commented Feb 2, 2018

Fixes at least one known case where this.has*Target property names would differ depending on locale. In Turkish, i is capitalized to İ so static targets = [ "input" ] would create a this.hasİnputTarget property for browsers using that locale, and this.hasInputTarget in others.

🙇‍♂️ to @rosa for tracking this down!

Further reading: What’s Wrong With Turkey?, Dotted and dotless I

Copy link
Contributor Author

@javan javan commented Feb 2, 2018

Example of the new test failing before adding the fix:

02 02 2018 15:18:40.771:INFO [Chrome 63.0.3239 (Windows 7 0.0.0)]: Connected on socket _RnZWId5hxKY3jY1AAAB with id 3045112
Chrome 63.0.3239 (Windows 7 0.0.0) TargetTests has*Target property names are not localized FAILED
	Expected: true
	Actual: undefined
	    at TargetTests.test has*Target property names are not localized (packages/@stimulus/core/test/index.ts:5469:21)
	    at TargetTests.<anonymous> (packages/@stimulus/core/test/index.ts:2533:52)
	    at step (packages/@stimulus/core/test/index.ts:2445:23)
	    at (packages/@stimulus/core/test/index.ts:2426:53)
	    at packages/@stimulus/core/test/index.ts:2420:71
	    at new Promise (<anonymous>)
Chrome 63.0.3239 (Windows 7 0.0.0): Executed 37 of 37 (1 FAILED) (1.196 secs / 1.083 secs)
@javan javan merged commit bb0b3d0 into master Feb 2, 2018
1 check passed
1 check passed
ci/circleci Your tests passed on CircleCI!
@javan javan deleted the stable-has-target-property-names branch Feb 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

1 participant