address.cityPrefix returning undefined on pt_BR

[ponei]

Describe the bug

Title pretty much sums it up. This is dependent on locale and doesn't happen on english - did not test on other locales other than pt_BR and en.

Reproduction

Calling address.cityPrefix or any other function that uses it (address.city sometimes does) returns undefined

Additional Info

No response

[Shinigami92]

https://github.com/faker-js/faker/blob/main/src/locales/pt_BR/address/city_prefix.ts

There is a city_prefix, but it is empty 🤔

Was this also the case with v5.5.3?

I will look into this after I fully woke up.

[ST-DDT]

FFR:

• https://github.com/faker-js/faker/blob/v5.5.3/lib/locales/pt_BR/address/city_prefix.js
• https://github.com/faker-js/faker/blob/v5.5.3/lib/locales/pt_BR/address/index.js#L3

[ST-DDT]

@ponei Are you fluent/familiar with pt_br?
If yes, could you create a PR that adds some common city prefixes?

[Shinigami92]

The only question I have right now is: was it different in v5.5.3, then it could be a || => ?? change somewhere or something like that. So maybe we accidentally changed behavior somewhere.

[ST-DDT]

The only question I have right now is: was it different in v5.5.3, then it could be a || => ?? change somewhere or something like that. So maybe we accidentally changed behavior somewhere.

This doesn't have any impact. An empty array wouldn't get replaced either way.

[Shinigami92]

We have a somewhat related PR for the general issue: #609

But adding some city prefixes for pt_BR would definitely be nice anyway

[ST-DDT]

Maybe add/fix citySuffix as well?

[ponei]

@ponei Are you fluent/familiar with pt_br?
If yes, could you create a PR that adds some common city prefixes?

Yes, I'm fluent with portuguese. I could come up with some after work, but prefixes for cities aren't very common here, suffixes are.
In english you would have something like West {cityName} but that doesn't translate really well.
Regions/other adjectives are suffixes because that's how they work in portuguese, so you would have something like {cityName} do Sul (south region).

Maybe the locale should say what it prefers? (suffix or prefix). Then if you want to override you could pass arguments explicitly saying otherwise.

[ST-DDT]

Yeah the generator patterns should be in the locale definitions as well (v6.2 or later).

Maybe add '' as prefix for now, that way you don't get undefined in those texts anymore.
The extra space in the city names is hopefully negligible.

[srjheam]

Maybe add/fix citySuffix as well?

There are no city prefixes/suffixes in pt_BR. Is there a way to return only address.cityName?

[Shinigami92]

For now please call faker.fake('{{address.cityName}}') as a workaround.

I looked into the implementation of

faker/src/address.ts

Lines 95 to 112 in 2b934c5

	city(format?: string | number): string {
	const formats = [
	'{{address.cityPrefix}} {{name.firstName}}{{address.citySuffix}}',
	'{{address.cityPrefix}} {{name.firstName}}',
	'{{name.firstName}}{{address.citySuffix}}',
	'{{name.lastName}}{{address.citySuffix}}',
	];
	if (!format && this.faker.definitions.address.city_name) {
	formats.push('{{address.cityName}}');
	}
	if (typeof format !== 'number') {
	format = this.faker.datatype.number(formats.length - 1);
	}
	return f(formats[format]);
	}

and damn, this is a mess 🤣
We need to fully rework this implementation, but this will be a breaking change. So I will move this to v7, because there is a workaround for now.

[ST-DDT]

Now there is faker.address.cityName() and faker.address.city() uses a localized pattern (not sure whether its correct though).
If its not correct, please open a PR and provide us with the correct one.

faker/src/locales/pt_BR/address/city.ts

Lines 2 to 5 in c5ac124

	'{{address.cityPrefix}} {{name.firstName}}{{address.citySuffix}}',
	'{{address.cityPrefix}} {{name.firstName}}',
	'{{name.firstName}}{{address.citySuffix}}',
	'{{name.lastName}}{{address.citySuffix}}',

The cityPrefix and suffix methods itself will be deprecated for removal in #1041 .