Skip to content
This repository has been archived by the owner on Dec 11, 2021. It is now read-only.

Add new package build with Rollup, dev environment with Storybook, & API #5

Merged

Conversation

thibaudcolas
Copy link
Contributor

This addresses some of the points raised in #4, in particular the ones related to the packaging of the code, and also adds an API.

I also made a fork of wagtail/wagtail#4942, which adds 3 commits to integrate this new version of the package: https://github.com/thibaudcolas/wagtail/commits/feature/new-streamfield-4942-build.

There is also https://github.com/thibaudcolas/wagtail/commits/feature/new-streamfield-4942-build-init, which contains one extra commit that explores changing how the widgets are initialised for better performance (gets rid of the time during which the page is loaded without StreamField instances)

@thibaudcolas thibaudcolas mentioned this pull request Jan 21, 2019
74 tasks
@@ -0,0 +1,8 @@
import { configure } from "@storybook/react";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using Storybook is probably the most opinionated of all the changes I made here. It would be possible to keep on using Webpack as before, but I think the improvements to the development environment are worth the addition of another tool, considering how simple it is to configure.

openAnalyzer: false,
logLevel: isProduction ? "info" : "warn",
}),
);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This plugin creates a report that shows the contents of the Webpack bundle, which is useful to understand how the bundle size is impacted by the package's code, and its dependencies. Note that this is a report for the dev/demo site, not for what is shipped to npm.

Example: http://demo.draftail.org/webpack-stats.html


storiesOf("React StreamField demo", module)
.addDecorator(story => <Provider store={store}>{story()}</Provider>)
.add("1 block type", () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are the demos from the index.html.

"last 1 Firefox version",
"last 2 iOS versions",
"last 2 Safari versions"
],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Configuration for Autoprefixer, matching Wagtail’s.

[
"@babel/preset-env",
{
"modules": false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This makes it so the ES modules -> CommonJS conversion is done by Rollup/Webpack, instead of Babel, which means those tools can do further optimisations. But Jest doesn't support ES modules, so we have to give it a specific config for the test env further below.

@@ -135,7 +135,8 @@ class StreamField extends React.Component {
const {id, generatedValue} = this.props;
return (
<DragDropContext onDragEnd={this.onDragEnd}>
<BlocksContainer fieldId={id} />
{generatedValue ? (<BlocksContainer fieldId={id} />) : null}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the potential fix for the react-redux v6 behavior change I mentioned in #4. But I ended up removing the v6 upgrade, so this isn't strictly needed. Thoughts welcome!

@BertrandBordage
Copy link
Contributor

This is excellent, and I love the Storybook integration you did! I’m happy to merge that, thank you.
I need to do a few changes right after the merge so that the path to Storybook gets used on github pages.

@BertrandBordage BertrandBordage merged commit 47f5a0a into wagtail-deprecated:master Feb 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants