Skip to content

Conversation

@eliihen
Copy link
Collaborator

@eliihen eliihen commented Jun 27, 2017

Why

Currently, Vagrant with the VirtualBox provider does not work with styleguidist, as there are issues with webpack's default behavior. The way around this is to make webpack use a polling scheme for checking for changed files. This is currently not configurable in styleguidist.

How

This PR makes the devServer.watchConfig configurable so that it is possible to use styleguidist with Vagrant

Fixes #515

@codecov-io
Copy link

codecov-io commented Jun 27, 2017

Codecov Report

Merging #516 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #516      +/-   ##
==========================================
+ Coverage   95.41%   95.42%   +<.01%     
==========================================
  Files          97       97              
  Lines        1266     1267       +1     
  Branches      262      262              
==========================================
+ Hits         1208     1209       +1     
  Misses         56       56              
  Partials        2        2
Impacted Files Coverage Δ
scripts/create-server.js 92.3% <100%> (+0.64%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8c68f01...320cc65. Read the comment docs.

stats: webpackConfig.stats || {},
});
{
watchOptions: webpackConfig.devServer ? webpackConfig.devServer.watchOptions : {},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this part isn’t required because webpack-merge should do recursive merge.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry, not quite sure what you mean here. Would you like to merge the entire webpackConfig.devServer into the styleguidist dev server config, not just watchConfig? Here I am checking for devServer being defined in the user config so it doesn't throw when it isn't.

I have not used webpack-merge before, so I am assuming it works like Object.assign in most cases

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’d try to remove this whole second block and see if it still works because webpack-merge does recursive merge and it should work ;-)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see! Thanks for the ELI5. It does seems to work without the last block because of the recursive merging.

docs/Cookbook.md Outdated
</div>
```

## How to use Vagrant with styleguidist?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

styleguidist → Styleguidist

docs/Cookbook.md Outdated

First of all, make sure you read [this guide](https://webpack.js.org/guides/development-vagrant/) from the webpack documentation.

The article mentions that you need to use `--watch-poll` when using the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’d write it shorter like this:

Then enable polling in your webpack config:

@sapegin
Copy link
Member

sapegin commented Jun 27, 2017

And thanks for the pull request! 🍕

This also means Vagrant is now supported
@eliihen
Copy link
Collaborator Author

eliihen commented Jun 27, 2017

Amended the commit with your requested changes

@sapegin sapegin merged commit 6dfcabf into styleguidist:master Jun 27, 2017
@eliihen eliihen deleted the issue-515 branch June 28, 2017 06:05
sapegin added a commit that referenced this pull request Jun 28, 2017
## New features

### Copy pathline to clipboard button

![](http://wow.sapegin.me/3t2u3S0P0L2T/Image%202017-06-28%20at%204.17.16%20PM.png)

(#485, #471 by @SaraVieira)

### Easier way to override style guide React components

New config option `styleguideComponents` to override React components used to render a style guide.

```javascript
module.exports = {
	styleguideComponents: {
		Logo: path.join(__dirname, 'styleguide/components/Logo'),
		StyleGuideRenderer: path.join(__dirname, 'styleguide/components/StyleGuide'),
	},
};
```

(#504)

### Customizable logging

New `logger` option:

```javascript
module.exports = {
	logger: {
    // One of: info, debug, warn
    // Suppress messages
		info: () => {},
    // Override display function
		warn: message => console.warn(`NOOOOOO: ${message}`),
	},
};
```

(#472, #510)

## Bug fixes

* Allow `devServer.watchConfig` to be configured, this also means Vagrant is now supported (#515, #516 by @esphen)
* Default `getExampleFilename` should be case-insensitive (#423, #460, #440, #482)
* Allow dynamic JSS styles
* Isolate search placeholder styles (#509, #491 by @n1313)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants