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

PR #1116 broke simple getExampleFilename customisation #1338

Closed
eemeli opened this issue Apr 16, 2019 · 4 comments
Closed

PR #1116 broke simple getExampleFilename customisation #1338

eemeli opened this issue Apr 16, 2019 · 4 comments

Comments

@eemeli
Copy link
Contributor

eemeli commented Apr 16, 2019

Current behavior

Previously, the documented getExampleFilename configuration allowed for just returning the path to where an examples file was expected to be found, but following #1116 a custom function now needs to return false or null if the file does not exist, because the fs.existsSync() check was removed from getExamples().

This change was not documented.

To reproduce

See example: https://github.com/eemeli/styleguidist-example/commits/get-examples

If you clone that repo, npm install && npm start from that branch will give you something like these errors:

./src/components/Button.js (./node_modules/react-styleguidist/lib/loaders/props-loader.js!./src/components/Button.js)
Module not found: Can't resolve '/Users/eemeli/code/rsg-example/src/components/Button.examples.md' in '/Users/eemeli/code/rsg-example/src/components'
./src/components/Placeholder.js (./node_modules/react-styleguidist/lib/loaders/props-loader.js!./src/components/Placeholder.js)
Module not found: Can't resolve '/Users/eemeli/code/rsg-example/src/components/Placeholder.examples.md' in '/Users/eemeli/code/rsg-example/src/components'
./src/components/PushButton.js (./node_modules/react-styleguidist/lib/loaders/props-loader.js!./src/components/PushButton.js)
Module not found: Can't resolve '/Users/eemeli/code/rsg-example/src/components/PushButton.examples.md' in '/Users/eemeli/code/rsg-example/src/components'

Expected behavior

Applying the changes of commit 882f8ee worked on previous versions of react-styleguidist. They should still work.

To fix, there are really two options here:

  1. Document the changed behaviour, and the user requirement for checking that the examples file exists before returning its path
  2. Re-add the check to getExamples().
@sapegin
Copy link
Member

sapegin commented Apr 18, 2019

I don't remember why it was removed ;-)

But we have a similar condition here:

hasExamples: examplesFile && fs.existsSync(examplesFile),

so probably adding it back won't break anything. Feel free to send a PR with a fix.

@eemeli
Copy link
Contributor Author

eemeli commented Apr 18, 2019

The code fix itself is pretty simple and I can do that, but updating the getExamples tests is trickier. Any guidance on how to mock the fs.existsSync calls there?

@sapegin
Copy link
Member

sapegin commented Apr 19, 2019

Historically we use real files in tests instead of mocking the file system. It's not ideal and I did mocking with https://github.com/streamich/memfs on another project. So both would be fine.

mendrew added a commit to mendrew/react-styleguidist that referenced this issue May 13, 2019
If file is not exist in fs we have errors during the build.
In this case we should continue to use default example file

iss: styleguidist#1338
sapegin pushed a commit that referenced this issue May 16, 2019
## New features

### Support React Hooks in examples

You can now use hooks in the examples, like the `useState` hook:

 ```jsx
const [count, setCount] = React.useState(42);
<Button onClick={() => setCount(count + 1)}>{count}</Button>
```

(#1353 by @eragon512)

## Bug fixes

* Accessibility fixes ([#1359](#1359) by @J-Kallunki)
* `getExampleFilename` should use default example if file does not exist ([#1362](#1362)) by @mendrew, closes [#1338](#1338)
* Hide HTML comments in Markdown files ([#1347](#1347) by @eragon512, closes #1326)
@styleguidist-bot
Copy link
Collaborator

🎉 This issue has been resolved in version 9.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

No branches or pull requests

3 participants