dynamicValues option does not work in browser

MH4GF commented 1 year ago

description

As the title says.

context

I use the generated functions mainly in storybook. When I activate dynamicValues and generate a function and start the storybook, I get the following error message.

ERROR in ./node_modules/casual/src/casual.js
Module not found: Error: Can't resolve 'fs' in '/src/my-org/my-repository/node_modules/casual/src'
 @ ./node_modules/casual/src/casual.js 2:13-26
 @ ./src/apis/graphql/mocks/builder.generated.ts
 @ ./src/components/MyComponent/index.stories.tsx
 @ ./src sync ^\.(?:(?:^|\/|(?:(?:(?!(?:^|\/)\.).)*?)\/)(?!\.)(?=.)[^/]*?\.stories\.(tsx|mdx))$
 @ ./generated-stories-entry.js
 @ multi ./node_modules/@storybook/core-client/dist/esm/globals/polyfills.js ./node_modules/@storybook/core-client/dist/esm/globals/globals.js (webpack)-hot-middleware/client.js?reload=true&quiet=false&noInfo=undefined ./storybook-init-framework-entry.js ./node_modules/@storybook/react/dist/esm/client/docs/config-generated-config-entry.js ./node_modules/@storybook/react/dist/esm/client/preview/config-generated-config-entry.js ./node_modules/@storybook/addon-docs/preview.js-generated-config-entry.js ./node_modules/@storybook/addon-actions/preview.js-generated-config-entry.js ./node_modules/@storybook/addon-backgrounds/preview.js-generated-config-entry.js ./node_modules/@storybook/addon-measure/preview.js-generated-config-entry.js ./node_modules/@storybook/addon-outline/preview.js-generated-config-entry.js ./node_modules/@storybook/addon-a11y/preview.js-generated-config-entry.js ./node_modules/@storybook/addon-knobs/dist/preset/addDecorator.js-generated-config-entry.js ./node_modules/storybook-addon-apollo-client/dist/preset/addDecorator.js-generated-config-entry.js ./.storybook/preview.tsx-generated-config-entry.js ./generated-stories-entry.js

casual is fs dependent and cannot run in your browser.

solutions

This is very troubling because of the dependent library issue. The following issues have been created in casual, but it does not seem to be possible to solve this problem because the fs module cannot be run on the web. https://github.com/boo1ean/casual/issues/20

Sorry I can't suggest a solution, but I created an issue to share with you all.

ardeois commented 1 year ago

@JimmyPaolini do you have an idea of how we could fix this?

JimmyPaolini commented 1 year ago

Hmm I'll look into it. I also noticed when I turned on dynamicValues in my repo there was a ReferenceError coming from the casual.uuid function.

JimmyPaolini commented 1 year ago

I see that casual hasn't been updated in over a year, and the maintainer no longer has time to maintain it: https://github.com/boo1ean/casual/issues/109

It might not be too hard to replace casual with another more popular/updated library like faker, what do you think?

ardeois commented 1 year ago

Dammit, I started this library with faker but switch to casual because faker was not maintained. Now I see it's the opposite 😓 https://fakerjs.dev/about/announcements/2022-01-14.html

Anyway it's a good idea to try switching to faker now that it's maintained properly

I'm sorry I don't have much time to work on the project right now, but contributions are always welcome ! @MH4GF or @JimmyPaolini would you be down for a PR?

MH4GF commented 1 year ago

I am interested in trying on the challenge of switching to faker👀 because the issue I want to solve can also be solved if dynamicValues is available in the browser. ref: #89

JimmyPaolini commented 1 year ago

I'm happy to help implement or review or test too! Once this is finished I can also work on expanding the dynamicValues feature to be more configurable than just on/off as it is now (i.e. implementing https://github.com/ardeois/graphql-codegen-typescript-mock-data/issues/89, https://github.com/ardeois/graphql-codegen-typescript-mock-data/issues/91)

ardeois commented 1 year ago

Great thank you @JimmyPaolini As you seem motivated to contribute to this repo, I've added you as a collaborator. You're the first 🎉 !

MH4GF commented 1 year ago

@ardeois @JimmyPaolini Let me discuss it with you. Breaking changes occur in the custom scalar generator, because the invocation methods of casual and faker are completely different. Added by this PR: https://github.com/ardeois/graphql-codegen-typescript-mock-data/pull/30

I think of several options, and we can transition in stages.

1: This shall remain a breaking change, use faker to provide similar feature and get users to migrate.
2: Coexistence of casual and faker, and use of casual only for custom scalar feature to maintain compatibility.
- While the ability to call casual with a custom scalar generator is not available in the browser, most of the functionality should be available while maintaining compatibility.
3: Casuals should be removed and prepare a map corresponding to casual method calls and make it available to faker.
- It may be possible to manage the generators of casual because there are not many of them.

I think that the number of users who use dynamicValues is still small, and it would not be good for the majority of users to be affected by the disruptive changes that affect them. Therefore, I think it would be better to proceed once with the second proposal, without making any breaking changes.

Remaining tasks are managed here: https://github.com/ardeois/graphql-codegen-typescript-mock-data/pull/92#issuecomment-1256863732

MH4GF commented 1 year ago

@ardeois @JimmyPaolini I implemented the second option as a trial. I would appreciate your opinion! https://github.com/ardeois/graphql-codegen-typescript-mock-data/pull/92

JimmyPaolini commented 1 year ago

Your implementation looks awesome! Since faker generates different fake data than casual even with the same seed number, won't this still possibly break people's tests if they upgrade, making it a breaking change? In which case, why not just remove all of casual and go with option 1 as you defined above?

ardeois commented 1 year ago

@MH4GF Awesome, thank you for your contribution ! I agree that option 2 is a better choice. We can still do a major release but keep compatibility of generated mocks for majority of users. I'll look into the PR soon

EDIT: I see you actually call faker for generated mocks as well. This would create a breaking change in the generated mock It's maybe okay but I'll discuss it more in the PR

ardeois commented 1 year ago

Fixed in #93 You can now use generateLibrary: 'faker' to switch generator

ardeois / graphql-codegen-typescript-mock-data