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.

Sign up for GitHub

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

Jump to bottom

Check whether complex objects should be frozen #2890

Closed
ST-DDT opened this issue May 11, 2024 · 3 comments · Fixed by #2928
Closed

Check whether complex objects should be frozen #2890

ST-DDT opened this issue May 11, 2024 · 3 comments · Fixed by #2928
Assignees
@Shinigami92
Labels
c: refactor PR that affects the runtime behavior, but doesn't add new features or fixes bugs good first issue Good for newcomers m: airline Something is referring to the airline module m: science Something is referring to the science module p: 1-normal Nothing urgent
Milestone
v9.0

Comments

@ST-DDT
Copy link
Member

ST-DDT commented May 11, 2024 

          Does freezing the locale entries here even make sense? `Object.freeze` only prevents the array instance from beeing mutated. Since all entries in the list are objects, wouldn't they profit more from being frozen instead?

Originally posted by @xDivisionByZerox in #2886 (comment)
@ST-DDT ST-DDT 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 m: science Something is referring to the science module m: airline Something is referring to the airline module labels May 11, 2024
@ST-DDT ST-DDT added this to the v9.0 milestone May 11, 2024
@ST-DDT ST-DDT added the breaking change Cannot be merged when next version is not a major release label May 11, 2024
@ST-DDT
Copy link
Member Author

ST-DDT commented May 11, 2024

Depending on how we decide here, this is a breaking change.

@ST-DDT
Copy link
Member Author

ST-DDT commented May 11, 2024

I prefer the data to be unfroozen or only being frozen on return. So that the raw data can be edited like any other data e.g. credit card issuers.

@ST-DDT ST-DDT changed the title Check whether complex objecta should be frozen Check whether complex objects should be frozen May 16, 2024
@ST-DDT
Copy link
Member Author

ST-DDT commented May 16, 2024

Team Decision

  • We will "unfreeze" all our data.

@ST-DDT ST-DDT added good first issue Good for newcomers and removed breaking change Cannot be merged when next version is not a major release s: needs decision Needs team/maintainer decision labels May 16, 2024
@Shinigami92 Shinigami92 self-assigned this May 27, 2024
@Shinigami92 Shinigami92 mentioned this issue May 27, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Shinigami92 Shinigami92

Labels
c: refactor PR that affects the runtime behavior, but doesn't add new features or fixes bugs good first issue Good for newcomers m: airline Something is referring to the airline module m: science Something is referring to the science module p: 1-normal Nothing urgent
Projects
None yet
Milestone
v9.0
Development

Successfully merging a pull request may close this issue.

refactor(locale): unfreeze arrays faker-js/faker
2 participants
@ST-DDT @Shinigami92