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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

webpack CLI #214

Merged
merged 29 commits into from Oct 18, 2016
Merged

webpack CLI #214

merged 29 commits into from Oct 18, 2016

Conversation

ovidiuch
Copy link
Member

@ovidiuch ovidiuch commented Oct 14, 2016

Big one 馃挘 cc @flaviusone @RadValentin @bogdanjsx @catalinmiron

New React Cosmos package: A CLI that makes integration as easy as possible for webpack users, using their existing webpack config and a new cosmos.config.js with just one required option (maybe zero in the future).

Please start by reading the Usage README section, it serves as a deliverable for this PR as well.

Other notable facts:

  • Started testing new packages with Jest. It's pretty cool & definitely faster. Will try to merge coverage reports between old and new suites in the future. Done
  • Changed the CP props a bit, separated components from fixtures.

@ovidiuch ovidiuch mentioned this pull request Oct 15, 2016
const nameMatcher = new RegExp(`^${fixturesRootPath}/(.+).(js|json)$`);
const matches = fixturePath.match(nameMatcher);
const name = matches[1];
const ext = matches[2];
Copy link
Member

Choose a reason for hiding this comment

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

Maybe path.extname along with path.basename can ease the work done here.

Copy link
Member Author

Choose a reason for hiding this comment

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

extname yes, but not basename, which a) contains the extension and b) is read from the last / to the right, whereas fixtures can be namespaced. E.g. __fixtures__/Button/simple-states/default.js, where matches[1] will be simple-states/default and basename will be default.js.

// the server and client side and reading it is different per case.
// On the server side we read the path from the webpack CLI params while on
// the client receives the embedded path through webpack.DefinePlugin.
// These function just applies the defaults, which are "universal".
Copy link
Member

Choose a reason for hiding this comment

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

This* function

@ovidiuch
Copy link
Member Author

cc @bogdanpetru you might want to take a look as well

return components;
}

export function prepareFixtures(modules, components) {
Copy link
Member

Choose a reason for hiding this comment

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

From my POV, each component should have their own folder

MyComponent/
  __fixtures__
    state_1.js
    state_2.js
  __tests__
    state_1.spec.js
    state_2.spec.js
  index.js
  index.less

Copy link
Member

Choose a reason for hiding this comment

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

This will also help you open sourcing the component with ease.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes! This has been on my mind for a while and has been raised by many people. I plan to approach this as well in the future, maybe even included in 1.0, but the current structure will need to continue to work as well.

This is where it can be integrated in the webpack CLI: https://github.com/skidding/react-cosmos/blob/1432aeb487aaae938d767f2de3e0b71309c23170/packages/react-cosmos-webpack/src/build-module-paths.js#L9

E.g.

// Current structure
const fixturesRootPathA = `${componentsRootPath}/__fixtures__/${componentCleanPath}`;
// Nested structure (like you proposed)
const fixturesRootPathB = `${componentsRootPath}/${componentCleanPath}/__fixtures__`;

I want to merge this PR before adding that functionality because I want to integrate Cosmos in a repo where it already works. If you want we can work together on adding nested fixture func in a subsequent PR. 馃憤

module.exports = {
devtool: 'eval',
resolve: {
extensions: ['', '.js', '.jsx'],
Copy link
Member

Choose a reason for hiding this comment

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

root: path.resolve('./path_to_root')

Reference: http://webpack.github.io/docs/configuration.html#resolve-modulesdirectories

This will remove the ../../../../../../constants/my_awesome_constant.js

Copy link
Member Author

Choose a reason for hiding this comment

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

This will remove the ../../../../../../constants/my_awesome_constant.js

Where do you see paths like this in the Flatris example?

@ovidiuch ovidiuch merged commit b2ef08d into master Oct 18, 2016
@ovidiuch ovidiuch deleted the 212-webpack-cli branch October 18, 2016 15:24
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.

None yet

3 participants