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

refactor: change location.state to return an object with formal state name, common name, abbreviation #2349

Open
ejcheng opened this issue Aug 27, 2023 · 5 comments
Labels
breaking change Cannot be merged when next version is not a major release c: refactor PR that affects the runtime behavior, but doesn't add new features or fixes bugs m: location Something is referring to the location module p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Milestone

Comments

@ejcheng
Copy link
Member

ejcheng commented Aug 27, 2023

Per internal team decision, we've decided to change location.state to return an object for each state with the state's full formal name, common name, and abbreviation. This change will be targeted for v9.

How should we best implement it gradually? Should we first deprecate the abbreviated boolean parameter on location.state?

Blocked by #1850

@ejcheng ejcheng added p: 1-normal Nothing urgent s: needs decision Needs team/maintainer decision c: refactor PR that affects the runtime behavior, but doesn't add new features or fixes bugs breaking change Cannot be merged when next version is not a major release m: location Something is referring to the location module labels Aug 27, 2023
@ejcheng ejcheng added this to the v9 - Next major milestone Aug 27, 2023
@ejcheng ejcheng changed the title refactor: change location.state return an object with formal state name, common name, abbreviation refactor: change location.state to return an object with formal state name, common name, abbreviation Aug 27, 2023
@matthewmayer
Copy link
Contributor

What if there was a new parameter like {object:true} to return the object instead of a string. Then it wouldn't be breaking.

@matthewmayer
Copy link
Contributor

(or a new method like faker.location.stateObject())

Might be worth the slightly more awkward naming to avoid breaking change on a very old & stable method.

@ST-DDT
Copy link
Member

ST-DDT commented Aug 31, 2023

Team Decision

We will change location.state() to return a complex object in v9.

@ST-DDT ST-DDT added s: accepted Accepted feature / Confirmed bug and removed s: needs decision Needs team/maintainer decision labels Aug 31, 2023
@matthewmayer
Copy link
Contributor

matthewmayer commented Sep 1, 2023

I think #1850 should be a prerequisite for this, because i imagine there's a fair amount of existing code like

const address = faker.helpers.fake("{{location.city}}, {{location.state}}") and there won't be an easy migration path for that if you can't access location.state.commonName or whatever

@matthewmayer
Copy link
Contributor

i'm still about dubious about changing the behavior of the existing methods. Sure, it will be in the migration guide, but people don't always read those, and subtle changes like this that don't issue deprecation warnings are easy to miss. You upgrade to v9, run your code, no warnings are shown, but then suddenly your location code starts outputting Detroit, [Object object] and you're not sure why. (ref #2288).

@ejcheng ejcheng added the s: on hold Blocked by something or frozen to avoid conflicts label Sep 4, 2023
@ST-DDT ST-DDT modified the milestones: v9.x, v9.0 Oct 3, 2023
@xDivisionByZerox xDivisionByZerox removed the s: on hold Blocked by something or frozen to avoid conflicts label Apr 29, 2024
@ST-DDT ST-DDT modified the milestones: v9.0, v10.0 Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Cannot be merged when next version is not a major release c: refactor PR that affects the runtime behavior, but doesn't add new features or fixes bugs m: location Something is referring to the location module p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Projects
None yet
Development

No branches or pull requests

4 participants