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
merged 1 commit into from Feb 2, 2018

Conversation

Projects
None yet
1 participant
@javan
Contributor

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

@javan

This comment has been minimized.

Contributor

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 Object.next (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

ci/circleci Your tests passed on CircleCI!
Details

@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