infra(ci): run against playground

[mshima]

Main types index.d.ts doesn’t exist.
Fix types to match '.' exports.

ref: jhipster/generator-jhipster#27179

[netlify]

✅ Deploy Preview for fakerjs ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	356e3ae
🔍 Latest deploy log	https://app.netlify.com/sites/fakerjs/deploys/66e03332d2c0220008ede2e5
😎 Deploy Preview	https://deploy-preview-3095.fakerjs.dev
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.97%. Comparing base (18ab2c7) to head (356e3ae).
Report is 18 commits behind head on next.

Additional details and impacted files

@@            Coverage Diff             @@
##             next    #3095      +/-   ##
==========================================
+ Coverage   99.96%   99.97%   +0.01%     
==========================================
  Files        2776     2776              
  Lines      226260   226260              
  Branches      945      591     -354     
==========================================
+ Hits       226183   226207      +24     
+ Misses         77       53      -24     

see 1 file with indirect coverage changes

[mshima]

Same fix is applied in #3094.
Alternative fix is applied in #3093 (removes types from root)

The above PRs tries to fix CommonJS types too, while this fixes esm types.

[Shinigami92]

you can checkout https://github.com/faker-js/playground and test your changes

[mshima]

you can checkout https://github.com/faker-js/playground and test your changes

Why don’t integrate playground on ci tests?

[Shinigami92]

you can checkout faker-js/playground and test your changes

Why don’t integrate playground on ci tests?

PR welcome, go and try for it

[Shinigami92]

@mshima thanks, this is a huge step forward and I wished for something like this a long time ❤️
you need to rebind/relink the repo faker to the playground test run
so something like modifying pnpm override before installing the playground deps

[mshima]

@Shinigami92 I am on mobile now, so I cannot rebase.
I think it’s only missing steps names now.
Feel free to modify the PR.

Playground is passing 🚀

[ST-DDT]

Please note that this will create a bi-directional dependency between two separate repositories.

[Shinigami92]

Please note that this will create a bi-directional dependency between two separate repositories.

Do you see any problem with this? IMO it's fine, because playground is in out org as well.

[xDivisionByZerox]

If we activate the playground for all PRs why would we need check-release-pr.yml anymore?

Do you see any problem with this? IMO it's fine, because playground is in out org as well.

We had a reason why we didn't do this previously. Now a random contributor might need to apply changes in the playground onto of the changes in faker when they work a breaking change feature.

[mshima]

IMO running playground in CI:

• reduces maintenance burden by avoiding regressions and requiring manual checks: fix: emit cts types #3093 (comment).
• improves contribution experience by not having to ask to manually check against playground: infra(ci): run against playground #3095 (comment)

We could do that but it has a side effect, that all features that are used in the playground that might experience breaking changes need to be updated to the new feature before it is available as a dependency. This is also the case for the releasenPR but that those are even rarer.

• the workflow can be ignored if it's not required to pass.
• contributor will be warned about breaking changes or unwanted breaking changes.

Moving playground to the main repository can be considered too.

[Shinigami92]

• the workflow can be ignored if it's not required to pass.
• contributor will be warned about breaking changes or unwanted breaking changes.

I agree 👍 we can mark the CI check as "not required" and if it breaks something, we are just "informed"

[ST-DDT]

I agree 👍 we can mark the CI check as "not required" and if it breaks something, we are just "informed"

Please note that that will only work once and will cause confusion for future PRs (that will fail the CI even if they didn't break anything because next is already "broken").

I would rather move the playgrounds inside the main repo to avoid these bidirectional dependencies.

[ST-DDT]

Team Decision

We don't want to add bi-directional dependencies between the playground and the faker repo.
We think that moving the playground to the main repo is the best solution for that, but that would require extensive work e.g. converting the repo into a mono-repo, what we don't want to do yet.

We would like to still run the pipeline on demand somehow e.g. using a comment, but we are currently unsure about the exact expectation we have for the pipeline. We will talk about this again in a future team meeting.

[ST-DDT]

If you have an idea, on how to move the playground to the main repo feel free to open a PR with a suggestion.

[ST-DDT]

We are looking into restructuring our project setup to solve this issue. For now, you don't have to update this PR.
Thanks for bringing this to our attention.

stale