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

dynamicValues option does not work in browser #90

Closed
MH4GF opened this issue Sep 23, 2022 · 12 comments · Fixed by #93
Closed

dynamicValues option does not work in browser #90

MH4GF opened this issue Sep 23, 2022 · 12 comments · Fixed by #93

Comments

@MH4GF
Copy link
Contributor

MH4GF commented Sep 23, 2022

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. boo1ean/casual#20

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

@ardeois
Copy link
Owner

ardeois commented Sep 23, 2022

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

@JimmyPaolini
Copy link
Collaborator

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
Copy link
Collaborator

JimmyPaolini commented Sep 23, 2022

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

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

@ardeois
Copy link
Owner

ardeois commented Sep 23, 2022

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
Copy link
Contributor Author

MH4GF commented Sep 23, 2022

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
Copy link
Collaborator

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 #89, #91)

@ardeois
Copy link
Owner

ardeois commented Sep 23, 2022

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
Copy link
Contributor Author

MH4GF commented Sep 24, 2022

@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: #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: #92 (comment)

@MH4GF
Copy link
Contributor Author

MH4GF commented Sep 25, 2022

@ardeois @JimmyPaolini
I implemented the second option as a trial. I would appreciate your opinion!
#92

@JimmyPaolini
Copy link
Collaborator

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
Copy link
Owner

ardeois commented Sep 26, 2022

@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
Copy link
Owner

ardeois commented Sep 29, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants