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

Deprecate datatype module entirely and move all methods into other modules #1590

Closed
Shinigami92 opened this issue Nov 22, 2022 · 5 comments
Closed
Labels
c: refactor PR that affects the runtime behavior, but doesn't add new features or fixes bugs deprecation A deprecation was made in the PR m: datatype Something is referring to the datatype module p: 1-normal Nothing urgent

Comments

@Shinigami92
Copy link
Member

We are adding the number module in #1122 and also #1341

Now we have some leftover methods in the datatype module but IMO it is not worth it to have a whole module for just these few methods.
I hope we can move these into other modules so we can get rid entirely of the datatype module.

@Shinigami92 Shinigami92 added c: refactor PR that affects the runtime behavior, but doesn't add new features or fixes bugs m: datatype Something is referring to the datatype module labels Nov 22, 2022
@Shinigami92 Shinigami92 moved this to Todo in Faker Roadmap Nov 22, 2022
@ST-DDT ST-DDT added p: 1-normal Nothing urgent deprecation A deprecation was made in the PR labels Nov 22, 2022
@ST-DDT
Copy link
Member

ST-DDT commented Dec 9, 2022

How about moving some stuff out of helpers as well?

  • faker.object or faker.record (NEW)
    • sample (basically faker.datatype.json() without jsonifing the output) <-- Not sure about that
    • sampleJson (datatype.json or maybe just drop it?) <-- Not sure about that
    • keyOf (helpers.objectKey)
    • valueOf (helpers.objectValue)
  • faker.array (NEW)
    • sample (datatype.array)
    • elementOf (helpers.arrayElement)
    • elementsOf (helpers.arrayElements)
    • of (helpers.multiple)
  • faker.date
    • sample (datatype.datetime)
  • faker.boolean (NEW)
    • sample (datatype.boolean) <-- Not sure about that

Maybe instead of ___Of we could use ___From.

@xDivisionByZerox
Copy link
Member

  • faker.object or faker.record (NEW)
    [...]
  • faker.array (NEW)

An Array is nothing more than a specific Object in JS, so no to faker.array. Furthermore, all modules (with helpers as exception) should be able to generate a value from a specific function without requiring any input. The proposed methods keyOf and valueOf always require an object as input. Thats why they ended up in helpers in the first place.

  • faker.boolean

I see why you propose this. But I have to say I really dislike having a module only for it to host one method. But to be fair, boolean is the one I'm unable to find a reasonable location as well.

@ST-DDT
Copy link
Member

ST-DDT commented Apr 6, 2023

Currently, only two methods are left:


datetime: We can easily move it to the date module.


boolean: We could:

  1. move it to helpers
    • ➖ Hard to find from code
  2. create a tiny boolean module
    • ➕ Easier to find from code
    • ➖ Only a single method for the entire module
  3. keep the tiny datatype module
    • ➖ Only a single method for the entire module
    • ➖ Unlikely to ever get more methods
  4. or do something else entirely.

There doesn't seem to be a perfect solution for this for now.
I like 2 more than 1 and 3.

@matthewmayer
Copy link
Contributor

I'd say a Boolean is a number 😀 have you considered faker.number.boolean?

@ST-DDT ST-DDT added the s: needs decision Needs team/maintainer decision label Apr 6, 2023
@ST-DDT
Copy link
Member

ST-DDT commented Apr 13, 2023

Team Decision

We will revive the datatype module with very simple methods for JS native datatypes only.

@ST-DDT ST-DDT closed this as completed Apr 13, 2023
@github-project-automation github-project-automation bot moved this from Todo to Done in Faker Roadmap Apr 13, 2023
@xDivisionByZerox xDivisionByZerox removed the s: needs decision Needs team/maintainer decision label Apr 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: refactor PR that affects the runtime behavior, but doesn't add new features or fixes bugs deprecation A deprecation was made in the PR m: datatype Something is referring to the datatype module p: 1-normal Nothing urgent
Projects
No open projects
Status: Done
Development

No branches or pull requests

4 participants