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.

Sign up for GitHub

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

Bugfix/fixing storybook venia ui #1945

Merged
merged 29 commits into from
Dec 6, 2019
Merged

Bugfix/fixing storybook venia ui #1945

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
60445b4
Creating a webpack base configuration in the buildpack package
LucasCalazans Oct 30, 2019
027ccff
Executing prettier
LucasCalazans Oct 30, 2019
4f0c509
Merge branch 'develop' into bugfix/fixing-storybook-venia-ui
Oct 31, 2019
a68b274
Importing data from venia-concept
LucasCalazans Nov 7, 2019
903702b
Merge branch 'bugfix/fixing-storybook-venia-ui' of github.com:LucasCa…
LucasCalazans Nov 7, 2019
a8826c9
Resolving conflicts with develop
LucasCalazans Nov 7, 2019
ff4d83b
Resolving conflicts part 2
LucasCalazans Nov 7, 2019
749acff
Merge branch 'develop' into bugfix/fixing-storybook-venia-ui
supernova-at Nov 11, 2019
c4b72e4
Merge remote-tracking branch 'origin/develop' into bugfix/fixing-stor…
supernova-at Nov 18, 2019
dc474fc
storybook:venia script working again
supernova-at Nov 18, 2019
410863a
Upgrades to Storybook 5.2.6
supernova-at Nov 18, 2019
794413e
Fixes BraintreeDropin story
supernova-at Nov 18, 2019
47191fe
Removes erroneous console.log
supernova-at Nov 18, 2019
45eeffa
Fixes Header component stories
supernova-at Nov 18, 2019
82fe5f4
Fixes Image component stories
supernova-at Nov 18, 2019
401fff0
PeregrineContextProvider and removes MiniCart stories
supernova-at Nov 18, 2019
a7ba81c
Merge branch 'develop' into bugfix/fixing-storybook-venia-ui
supernova-at Nov 20, 2019
7795f61
Resolving conflicts
LucasCalazans Nov 24, 2019
2ff60c9
Updating yarn.lock after merge conflict
LucasCalazans Nov 24, 2019
38544d6
Adding some improvements by PR review
LucasCalazans Nov 25, 2019
1c434cd
Fixing prettier errors
LucasCalazans Nov 25, 2019
62de623
Using yarn workspace to storybook script
LucasCalazans Nov 26, 2019
cb4526b
Merge branch 'develop' into bugfix/fixing-storybook-venia-ui
LucasCalazans Nov 26, 2019
d195d5e
Merge branch 'develop' into bugfix/fixing-storybook-venia-ui
revanth0212 Nov 27, 2019
1e0a4f0
Use loadEnvironment to obtain references to env variables from venia-…
sirugh Dec 3, 2019
9eca071
Merge branch 'develop' into bugfix/fixing-storybook-venia-ui
supernova-at Dec 4, 2019
ae9f529
Merge remote-tracking branch 'origin/develop' into pr/LucasCalazans/1945
tjwiebell Dec 5, 2019
b37839d
Change function signature back
tjwiebell Dec 6, 2019
e224820
Add comments and mode
sirugh Dec 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions packages/venia-ui/.storybook/backendUrl.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
const envVarDefinitions = require('@magento/pwa-buildpack/envVarDefinitions.json');

const backendUrlSection = envVarDefinitions.sections.find(section =>
section.variables.find(variable => variable.name === 'MAGENTO_BACKEND_URL')
);

module.exports = backendUrlSection.variables[0].example;
22 changes: 20 additions & 2 deletions packages/venia-ui/.storybook/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,26 @@
import { configure } from '@storybook/react';
import React from 'react';
import { configure, addDecorator } from '@storybook/react';
import { AppContextProvider } from '../lib/components/App';
import { Adapter } from '@magento/venia-drivers';
import store from '@magento/venia-concept/src/store';
import backendUrl from './backendUrl';
import '@magento/venia-concept/src/index.css';

function loadStories() {
const context = require.context('../src', true, /__stories__\/.+\.js$/);
const context = require.context('../lib', true, /__stories__\/.+\.js$/);
context.keys().forEach(context);
}

const apiBase = new URL('/graphql', backendUrl).toString();

addDecorator(storyFn => (
<Adapter
apiBase={apiBase}
supernova-at marked this conversation as resolved.
Show resolved Hide resolved
apollo={{ link: Adapter.apolloLink(apiBase) }}
store={store}
>
<AppContextProvider>{storyFn()}</AppContextProvider>
</Adapter>
));

configure(loadStories, module);
23 changes: 20 additions & 3 deletions packages/venia-ui/.storybook/webpack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,25 @@
const base_config = require('../webpack.config.js');
const {
graphQL: { getUnionAndInterfaceTypes }
} = require('@magento/pwa-buildpack');
const baseWebpackConfig = require('@magento/venia-concept/webpack.config');
const { DefinePlugin } = require('webpack');
const backendUrl = require('./backendUrl');

module.exports = async storybookBaseConfig => {
const conf = await base_config();
module.exports = async (storybookBaseConfig, mode, env) => {
process.env.MAGENTO_BACKEND_URL = backendUrl;

const webpackData = await baseWebpackConfig(env);
const conf = webpackData[0];

const unionAndInterfaceTypes = await getUnionAndInterfaceTypes();
storybookBaseConfig.module = conf.module;
storybookBaseConfig.resolve = conf.resolve;
storybookBaseConfig.plugins = [
...storybookBaseConfig.plugins,
new DefinePlugin({
UNION_AND_INTERFACE_TYPES: JSON.stringify(unionAndInterfaceTypes)
})
];

return storybookBaseConfig;
};
1 change: 0 additions & 1 deletion packages/venia-ui/lib/components/Checkout/__stories__/braintreeDropin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import React from 'react';
import { storiesOf } from '@storybook/react';

import BraintreeDropin from '../braintreeDropin';
import '../../../index.css';

const stories = storiesOf('Checkout/BraintreeDropin', module);

Expand Down
28 changes: 2 additions & 26 deletions packages/venia-ui/lib/components/Header/__stories__/header.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,38 +3,14 @@ import { storiesOf } from '@storybook/react';

import Header from '../header';
import defaultClasses from '../header.css';
import '../../../index.css';
import { Adapter } from '@magento/venia-drivers';
import store from '../../../store';

const stories = storiesOf('Header', module);
const apiBase = new URL('/graphql', location.origin).toString();
const noop = () => {};

stories.add('Search Bar Closed', () => (
<Adapter
apiBase={apiBase}
apollo={{ link: Adapter.apolloLink(apiBase) }}
store={store}
>
<Header
classes={defaultClasses}
searchOpen={false}
toggleSearch={noop}
/>
</Adapter>
<Header classes={defaultClasses} searchOpen={false} toggleSearch={noop} />
));

stories.add('Search Bar Open', () => (
<Adapter
apiBase={apiBase}
apollo={{ link: Adapter.apolloLink(apiBase) }}
store={store}
>
<Header
classes={defaultClasses}
searchOpen={true}
toggleSearch={noop}
/>
</Adapter>
<Header classes={defaultClasses} searchOpen={true} toggleSearch={noop} />
));
2 changes: 0 additions & 2 deletions packages/venia-ui/lib/components/Image/__stories__/image.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import React from 'react';
import { storiesOf } from '@storybook/react';

import '../../../index.css';
import Image from '../image';
import classes from './image.css';

const stories = storiesOf('Image', module);

stories.add('An Image using a direct src', () => (
Expand Down
45 changes: 4 additions & 41 deletions packages/venia-ui/lib/components/MiniCart/__stories__/MiniCart.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,9 @@
import React from 'react';
import { storiesOf } from '@storybook/react';

import '../../../index.css';
import { Adapter } from '@magento/venia-drivers';
import store from '../../../store';

import MiniCart from '../miniCart';
import defaultClasses from '../miniCart.css';

const apiBase = new URL('/graphql', location.origin).toString();
const noop = () => {};

const nonConfigurableItem = {
Expand Down Expand Up @@ -83,31 +78,15 @@ stories.add('Loading', () => {
}
};

return (
<Adapter
apiBase={apiBase}
apollo={{ link: Adapter.apolloLink(apiBase) }}
store={store}
>
<MiniCart {...props} />
</Adapter>
);
return <MiniCart {...props} />;
});

stories.add('Empty Cart', () => {
const props = {
...baseProps
};

return (
<Adapter
apiBase={apiBase}
apollo={{ link: Adapter.apolloLink(apiBase) }}
store={store}
>
<MiniCart {...props} />
</Adapter>
);
return <MiniCart {...props} />;
});

stories.add('Many Items', () => {
Expand All @@ -134,15 +113,7 @@ stories.add('Many Items', () => {
isCartEmpty: false
};

return (
<Adapter
apiBase={apiBase}
apollo={{ link: Adapter.apolloLink(apiBase) }}
store={store}
>
<MiniCart {...props} />
</Adapter>
);
return <MiniCart {...props} />;
});

stories.add('Editing', () => {
Expand All @@ -157,13 +128,5 @@ stories.add('Editing', () => {
isCartEmpty: false
};

return (
<Adapter
apiBase={apiBase}
apollo={{ link: Adapter.apolloLink(apiBase) }}
store={store}
>
<MiniCart {...props} />
</Adapter>
);
return <MiniCart {...props} />;
});
7 changes: 6 additions & 1 deletion packages/venia-ui/lib/components/SearchBar/__stories__/searchBar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,10 @@ import defaultClasses from '../searchBar.css';
const stories = storiesOf('SearchBar', module);

stories.add('Search Bar', () => (
<SearchBar classes={defaultClasses} isOpen={true} />
<SearchBar
classes={defaultClasses}
isOpen={true}
location={{}}
history={{ push: () => {} }}
LucasCalazans marked this conversation as resolved.
Show resolved Hide resolved
/>
));
5 changes: 4 additions & 1 deletion packages/venia-ui/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@
"peerDependencies": {
"@magento/babel-preset-peregrine": "~1.0.0",
"@magento/peregrine": "~4.0.0",
"@magento/pwa-buildpack": "~4.0.0",
"@magento/venia-concept": "~4.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

The idea is that peregrine is used by venia-ui is used by venia-concept. This seems like a red flag to me? But maybe I'm misunderstanding peer deps :P

I think this, along with the imports of venia-concept files in venia-ui just seem like red flags to me in terms of where we are supposed to be keeping these things. Maybe in the future we will move the majority of the base config to the top-level and venia-concept and venia-ui/storybook can just import/use it.

TL;DR - I think I'm fine with this, just rambling.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I think for the storefront app that's correct. For Storybook we relaxed the rules a bit so we could run it easily.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, it's a tough call. I do think @sirugh has the right of it here.

It's true that venia-ui components are hard to render without the glue that venia-concept adds, so we need to get that glue from somewhere. But venia-ui is built to be a dependency of venia-concept, so I'd prefer that it didn't list venia-concept as a dependency or import anything from it. Thinking about our various options here:

Option Evaluation
Duplicate store.js, webpack.config.js, and index.css bad
Move these files up to some shared package bad
Import these files from venia-concept not ideal
Move venia-ui storybook stories into venia-concept okay?

Overall, I would accept the approach in this PR because the work's been put in, it has value, and only the Storybook files are importing from venia-concept. But eventually I want to remove the circular dependency.

"apollo-cache-inmemory": "~1.4.3",
"apollo-cache-persist": "~0.1.1",
"apollo-client": "2.6.4",
Expand All @@ -83,7 +85,8 @@
"react": "~16.9.0",
"react-redux": "~7.1.1",
"react-router-dom": "~5.1.0",
"redux": "~4.0.1"
"redux": "~4.0.1",
"webpack": "~4.41.2"
},
"sideEffects": false
}