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

Make Faker.js a user-configured dependency #1037

Closed
Gaurav0 opened this issue Mar 8, 2017 · 36 comments
Closed

Make Faker.js a user-configured dependency #1037

Gaurav0 opened this issue Mar 8, 2017 · 36 comments
Labels

Comments

@Gaurav0
Copy link
Contributor

Gaurav0 commented Mar 8, 2017

No description provided.

@oskarrough
Copy link

See #1062

@fran-worley
Copy link

As a side point, could we look at making the faker an user configured add on.

That would mean that there is no hard dependency to a faker version and people would have better control over its configuration. E.g. requiring specific locales for example.

@samselikoff
Copy link
Collaborator

Yes, in retrospect perhaps it shouldn't be in dependencies, but should be added as a devDependency to the host app's package.json. One issue is as people share upstream, they're now also responsible for declaring that dependency...

@acorncom
Copy link
Collaborator

acorncom commented Oct 6, 2017

As discussed at #1062, bumping the version on Faker gets a bit more complicated, as the source files we currently rely on are removed in the new version. Upgrading will require working through that now ...

johno/ember-faker#15 might be worth referencing, though I haven't looked at it in detail ...

@caseywatts caseywatts mentioned this issue Nov 8, 2017
@caseywatts
Copy link
Contributor

Our options include:

  1. getting faker 4 to distribute the built js file to npm like it used to thread
  2. deal with using the source code from faker directly somehow. Something like: working out how to get our build tools to slurp in a commonjs module as if it's an es6 module? (idk if I'm saying things accurately - no idea what's going on with our build tools lol)

Separately: even if we want this dependency to move out of ember-cli-mirage and into something like ember-faker, it will have to deal with this same bump, too. (it's still on 3.0.0](https://github.com/johnotander/ember-faker/blob/5be619702d9fe8ea5aedd700f7d87f4414a3f1b8/package.json#L51))

@caseywatts
Copy link
Contributor

I got faker to export the main file faker.js again 🎉 (my option 1 above)
https://github.com/Marak/faker.js/issues/575

@caseywatts
Copy link
Contributor

caseywatts commented Dec 2, 2017

https://github.com/Marak/faker.js/pull/584 it's merged, we're waiting on a release now

@caseywatts
Copy link
Contributor

It's been released since then - ready for us to do the change we need on this side :)

@samselikoff
Copy link
Collaborator

@caseywatts so we can bump now? Are there any breaking changes?

@mehulkar
Copy link
Contributor

mehulkar commented Apr 10, 2018

I don't think a new version has been published since @caseywatts's PR (it's after the 4.1.0 release). Also looks like a new version isn't going to be published: https://github.com/Marak/faker.js/issues/621#issuecomment-376164148

@Leooo
Copy link
Contributor

Leooo commented Apr 30, 2018

suddenly getting a build error when using this Faker version: Build Error (Babel)

identity/bower_components/Faker/examples/node/minimal-usage.js: 'return' outside of function (9:0)

@samselikoff
Copy link
Collaborator

@Gaurav0
Copy link
Contributor Author

Gaurav0 commented Aug 7, 2018

Recommend publishing a @mirage/faker fork

@mehulkar
Copy link
Contributor

mehulkar commented Aug 7, 2018

I'd say just take it out entirely and update the docs to recommend that users install Faker (or an equivalent) separately.

It would be totally fine to also publish a fork in parallel as @Gaurav0 suggests, but I I'd love Mirage just the same even if it didn't install Faker for me.

@samselikoff
Copy link
Collaborator

Ya. Leaning towards taking it out since we have auto-import now, but it is nice having it in there.

What would @mirage/faker fork look like?

@Gaurav0
Copy link
Contributor Author

Gaurav0 commented Aug 8, 2018

At least the current master, if not accepting some PRs that have been ignored.

@samselikoff
Copy link
Collaborator

Hm... that seems like a big maintenance burden.

At this point I think the options are

  1. Relying on Faker as a peer dependency so folks can configure it independently of their Mirage version, but it still comes "out of the box" with Mirage, or

  2. Dropping Faker from Mirage entirely and providing some instructions on how to easily install & use it alongside Mirage, via ember-auto-import & the source npm package

@rtablada
Copy link

rtablada commented Aug 8, 2018

I would 😍 using faker at the app level rather than as a dependency of Mirage.

I think peerDependency for now makes sense with the instructions that @ef4 mentioned in the discuss link posted above.

Seems best with a few 👍 side-effects:

  1. Auto Import Use for some dependency shaking
  2. User land can use whatever version of faker they want (assuming maybe a min major version)
  3. User's can import faker from 'faker' ❤️
  4. Possibly remove any use of faker internally from mirage down the road

I don't think that we need to go straight to "Dropping Faker from Mirage entirely" unless it's as small as one reference.

There's also a lot of import { faker } from 'ember-cli-mirage' out there and ideally there would be an upgrade path (and maybe even a small codemod) so that CLI doesn't just break on those import statements.

@samselikoff
Copy link
Collaborator

Yea a codemod would be great for that.

I'm thinking peerDependency doesn't actually make sense here, since Mirage doesn't depend on faker, it just shipped with it so it was easily available for client apps. But now with auto-import it's basically just as easy.

I think eaf's peerDependency workflow makes sense for ECLI-Tailwind & the tailwind npm package.

@caseywatts
Copy link
Contributor

I just started a codemod here:
https://github.com/caseywatts/ember-cli-mirage-faker-codemod

(not usable yet, just some skeleton-y code & some reference material I found useful)

@samselikoff
Copy link
Collaborator

Amazing! I would love to button this up and include a reference to it as part of v1.0 (still a little bit away, but not too far).

What do you think?

@caseywatts
Copy link
Contributor

I'd love to share this there, yes :)

It's not working yet lol - but I don't mind if you point at it even while it's a work-in-progress, no problem. It could be a starting point for someone else to finish it if they have time sooner than I do. I'm trying to document it enough that someone could pick it up at least.

@caseywatts
Copy link
Contributor

caseywatts commented Aug 23, 2018

This works now! :D

https://github.com/caseywatts/ember-cli-mirage-faker-codemod

It could use a couple more things, but they're optional:

@caseywatts
Copy link
Contributor

caseywatts commented Aug 28, 2018

Run the codemod

The codemod is all set!

npm install -g jscodeshift
jscodeshift -t https://raw.githubusercontent.com/caseywatts/ember-cli-mirage-faker-codemod/master/transform.js ./tests

Faker

Getting faker to install using ember-auto-import is another issue though. When trying to use it, I get faker is undefined.

Issues

  1. Faker's branch hasn't been released in a long time, and it's not clear when it will be - see this issue comment. We may want to fork faker, or recommend using an older version (from before the build file was removed).
  2. Even when that release is done, ember-auto-import doesn't successfully auto-import it. I made my own fork of faker to test against, and it's still undefined. The export makes it to node_modules/faker/build/build/faker.js but that's probably not somewhere ember-auto-import is looking.

Install faker and ember-auto-import

yarn add https://github.com/caseywatts/faker.js/
git add . && git commit -m "adding faker package"
ember install ember-auto-import
git add . && git commit -m "installing ember-auto-import package"

@caseywatts
Copy link
Contributor

got it! ✨

  1. Faker has released a version since then, and it has build/build/faker.js in there
  2. It doesn't work as beautifully as I hoped, but we can give autoImport a hint to find the built file (below)

Install faker and ember-auto-import

yarn add faker
git add . && git commit -m "adding faker package"
ember install ember-auto-import
git add . && git commit -m "installing ember-auto-import package"

add to ember-cli-build.js:

    autoImport: {
      alias: {
        'faker': 'build/build/faker'
      }
    }

@AndreJoaquim
Copy link

@caseywatts, thank you very much for all the work you put in this topic.

I think that it would be good to provide a small tutorial/guide/tip on how to remove faker from production builds, which is a feature that ember-cli-mirage will stop providing

@caseywatts
Copy link
Contributor

@AndreJoaquim good idea! I think your idea is to help people remove faker from the ember-cli-mirage build itself - especially for people who have no plans to use faker at all, even directly from npm.

Could you make a new issue about that so it gets tracked better? :)

@caseywatts
Copy link
Contributor

I just updated the instructions from this thread into the README over here:
https://github.com/caseywatts/ember-cli-mirage-faker-codemod

@Gaurav0
Copy link
Contributor Author

Gaurav0 commented Oct 30, 2018

Can someone provide instructions on how to import without ember-auto-import? Just using app.import with the correct path and generating a vendor-shim works fine.

app.import('node_modules/faker/build/build/faker.js');

ember generate vendor-shim faker

@samselikoff
Copy link
Collaborator

samselikoff commented Jan 15, 2019

After chatting with @rwjblue, I think what I'm going to do here (as well as for the pretender issue) is allow host apps to specify their own version, while still keeping the packages in Mirage's dependencies. That way if folks don't care about specifying their own versions, they aren't responsible for them.

To do this we can copy ember-jquery's code here. This will replace the pretender and faker import lines here.

This also lets us do this in a forwards-compatible way, which I didn't think we'd be able to do. All that's left is to add documentation on how folks can go about specifying their own versions of faker and pretender, if & when they have the need.

@samselikoff
Copy link
Collaborator

samselikoff commented Jan 15, 2019

I actually might remove faker as a dep/peerDep at all, its not really a dep, and we can leverage Casey's wonderful codemod and provide instructions for how to install.

I'll still use the above approach for Pretender.

@jelhan
Copy link
Contributor

jelhan commented Jan 24, 2019

I actually might remove faker as a dep/peerDep at all, its not really a dep, and we can leverage Casey's wonderful codemod and provide instructions for how to install.

IMO that's the best solution. It would also make it much easier to use another library to generate random data for testing. I have never used it myself, but e.g. Chance looks promising. Please note that you already listed dropping faker as a to do for releasing v1.0.0 on August 2018 here #1363.

@samselikoff samselikoff changed the title Bump faker to 4.0 Make Faker.js a user-configured dependency Jan 24, 2019
@samselikoff
Copy link
Collaborator

@jelhan FWIW you could absolutely install Chance, import with Ember Auto Import, and use it in your Mirage factories today. No need to use Faker at all.

@devinus
Copy link

devinus commented Feb 8, 2019

Another issue is that the Faker included with Mirage seems to break source maps on Chrome when excludeFilesFromBuild: false.

> SyntaxError: Unexpected token თ in JSON at position 3573855", source: chrome-devtools://devtools/bundled/shell.js (5566)

Related: https://github.com/Marak/faker.js/issues/375

@samselikoff
Copy link
Collaborator

#1527 removes faker!

I added upgrade notes to the AddonDocs site and will copy them over to the Release, once we cut 1.0.

I tried this out on EmberMap's frontend but ran into issues with @caseywatts' codemod. I'm going to wait until we get that sorted out & working before releasing 1.0 (among a few other things).

@samselikoff
Copy link
Collaborator

75c458d adds docs with codemod & closes.

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

No branches or pull requests