feat: only generate IANA example domains - #3967
Conversation
✅ Deploy Preview for fakerjs ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
ST-DDT
left a comment
ST-DDT
left a comment
There was a problem hiding this comment.
CI is failing. Please run pnpn run preflight before commiting.
Closes faker-js#2228. Add an optional `{ allowOnlyExamples?: boolean }`: when true, returns <word>.example.<com|org|net>: a 3 label subdomain under the IANA reserved example domains (RFC 2606), which never resolve. Default (no option / false) behavior is unchanged.
f2d5f92 to
a6d3a03
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #3967 +/- ##
=======================================
Coverage 98.91% 98.91%
=======================================
Files 923 924 +1
Lines 3223 3228 +5
Branches 567 586 +19
=======================================
+ Hits 3188 3193 +5
Misses 31 31
Partials 4 4
🚀 New features to boost your workflow:
|
ST-DDT
added
c: feature
There was a problem hiding this comment.
Pull request overview
This PR adds an opt-in way to generate domain names under IANA-reserved “example” domains to reduce the chance of producing domains that might be mistaken for real sites, addressing #2228.
Changes:
- Extend
internet.domainName()with an optional{ allowOnlyExamples?: boolean }parameter that produces<word>.example.<com|org|net>when enabled. - Update seeded internet module tests to cover both the default
domainName()call and the new option. - Refresh snapshots to reflect the new test structure and the example-domain output.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test/modules/internet.spec.ts | Adds a dedicated domainName test group to exercise the new option. |
| test/modules/snapshots/internet.spec.ts.snap | Updates snapshot keys/values for the new domainName cases. |
| src/modules/internet/module.ts | Implements allowOnlyExamples behavior and documents the new option. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * @param options The optional options object. | ||
| * @param options.allowOnlyExamples When `true`, generate a domain under the IANA-reserved example domains (example.com / example.org / example.net) so the result never resolves to a real host. See issue #2228. | ||
| * |
| /** | ||
| * When `true`, generate a domain under the IANA-reserved example domains (example.com / example.org / example.net) so the result never resolves to a real host. | ||
| */ |
|
I'm unsure what the best way to achieve this is. Implementation is mostly fine, but I won't comment on the remaining todos until we decide on the solution. |
| if (options?.allowOnlyExamples) { | ||
| const word = this.domainWord(); | ||
| const tld = this.faker.helpers.arrayElement(['com', 'org', 'net']); | ||
| return `${word}.example.${tld}`; | ||
| } |
There was a problem hiding this comment.
Maybe this could be replaced by a more flexible baseDomain/parentDomain parameter.
example.com resolves to a real host, so the current claim is not really true.
We could still mention the three example domains in the jsdocs of the new parameter.
What do you think?
There was a problem hiding this comment.
I like
internet.domainName({baseDomain:"example.com"})
As it unlocks more use cases like
internet.domainName({baseDomain:"co.uk"})
internet.domainName({baseDomain:"yourdomain.com"})
internet.domainName({baseDomain:"example"})
Closes #2228.
Add an optional
{ allowOnlyExamples?: boolean }: when true, returns .example.<com|org|net>: a 3 label subdomain under the IANA reserved example domains (RFC 2606), which never resolve. Default (no option / false) behavior is unchanged.