-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[Tooling] yarn splash (beta)
#2113
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
Changes from all commits
08b6744
44da583
9912178
9d511ba
3adc842
a9da56e
be17151
b0c0503
90801ff
5fde385
acd9c2e
370d844
8cb4113
f7bfb82
61b233e
e50011a
204bb60
7ce1a5e
f10ffd3
7493e70
fb5019b
0db5a4b
72bc4ac
230d07c
961e3ae
4028392
af429e9
f397782
9d1873c
9b081de
3502aef
4a53045
aed6ff3
ec0fb8c
12e60e6
4a575dd
7dd8e0a
7eeba2a
768957f
1863788
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| { | ||
| "rules": { | ||
| "jsx-a11y/accessible-emoji": "off", | ||
| "shopify/jsx-no-hardcoded-content": "off" | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| # `yarn splash` (beta) | ||
|
|
||
| `yarn splash` is a command-line interface to observe the splash zone of a change across the component library. | ||
|
|
||
| It answers the question: | ||
|
|
||
| > When I modify a component, what parts of the system did that also touch (also known as the change’s “splash zone”)? | ||
|
|
||
| ## How to use `yarn splash` | ||
|
|
||
| 1. Edit files in `src/`, such as components 🧩 and style sheets 🎨 | ||
| 2. Run `yarn splash` to see the splash zone of your changes in the working directory | ||
|
|
||
| 💡 Tip: <kbd>command</kbd> + click a file path to open it in your text editor | ||
|
|
||
| ## Feedback and bug reports | ||
|
|
||
| Found an issue or want to share feedback? | ||
|
|
||
| Reach out on Slack in [#polaris-tooling](https://shopify.slack.com/messages/CCNUS0FML). |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,200 @@ | ||
| import path from 'path'; | ||
| import React, {useState, useEffect} from 'react'; | ||
| import {Box, Text, Color, render} from 'ink'; | ||
| import sortBy from 'lodash/sortBy'; | ||
| import {getGitStagedFiles, getDependencies} from './treebuilder'; | ||
|
|
||
| const excludedFileNames = (fileName) => | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think these checks are a bit too lose. For example,
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I agree. As we make it more codebase-agnostic, this won't scale. |
||
| !fileName.includes('test') && !fileName.includes('types'); | ||
|
|
||
| const getEmojiForExtension = (extension) => { | ||
| switch (extension) { | ||
| case '.tsx': | ||
kaelig marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| case '.ts': | ||
| return '🧩'; | ||
| case '.scss': | ||
| return '🎨'; | ||
| default: | ||
| return '❔'; | ||
| } | ||
| }; | ||
|
|
||
| const formatDependencies = (dependencies) => | ||
| dependencies | ||
| .filter(({fileName}) => excludedFileNames(fileName)) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you think we should filter here too? I think, for example, if you made a change to
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right. Let me put those index and utils back in. If feedback says this is too overwhelming we can always filter them out again. |
||
| .map((dependency) => ({ | ||
| pathname: `${path.dirname(dependency.fileName)}/`, | ||
| filename: path.basename(dependency.fileName), | ||
| dependencies: sortBy( | ||
| dependency.dependencies.filter(excludedFileNames).map((dependency) => ({ | ||
| pathname: `${path.dirname(dependency)}/`, | ||
| filename: path.basename(dependency), | ||
| componentName: path | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This might not necessarily be a component
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll have to think about those edge cases, yeah. |
||
| .dirname(dependency) | ||
| .replace('src/components/', '') | ||
| .split('/')[0], | ||
| })), | ||
| ['pathname', 'filename'], | ||
| ), | ||
| })); | ||
|
|
||
| const Component = ({pathname, filename, dependencies}) => ( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please define these components (and really all top-level functions) using the |
||
| <Box marginBottom={1} flexDirection="column"> | ||
| <Box> | ||
| <Box width={3}>{getEmojiForExtension(path.extname(filename))}</Box> | ||
| <Text bold> | ||
| <Color greenBright> | ||
| {pathname} | ||
| {filename} | ||
| </Color> | ||
| </Text> | ||
| </Box> | ||
| <Box marginLeft={3}> | ||
| <Box width={23}>Component name</Box> | ||
| Files potentially affected (total: {dependencies.length}) | ||
| </Box> | ||
| {dependencies.map(({pathname, filename, componentName}) => ( | ||
| <Box | ||
| marginLeft={3} | ||
| key={pathname + filename} | ||
| flexDirection={process.stdout.columns > 80 ? 'row' : 'column'} | ||
| > | ||
| <Box width={23}> | ||
| <Color dim>{'<'}</Color> | ||
| <Color>{componentName}</Color> | ||
| <Color dim>{' />'}</Color> | ||
| </Box> | ||
| <Box textWrap="wrap"> | ||
| <Color dim>{pathname}</Color> | ||
| {filename} | ||
| </Box> | ||
| </Box> | ||
| ))} | ||
| </Box> | ||
| ); | ||
|
|
||
| const Components = ({components, status}) => ( | ||
| <React.Fragment> | ||
| {status === 'loading' && ( | ||
| <Box marginLeft={4} marginBottom={1}> | ||
| ⏳{' '}Please wait during compilation… Beep boop beep 🤖 | ||
| </Box> | ||
| )} | ||
|
|
||
| {status === 'loaded' && | ||
| components.map(({pathname, filename, dependencies}) => ( | ||
| <Component | ||
| key={pathname + filename} | ||
| pathname={pathname} | ||
| filename={filename} | ||
| dependencies={dependencies} | ||
| /> | ||
| ))} | ||
| </React.Fragment> | ||
| ); | ||
|
|
||
| const Summary = ({ | ||
| componentsModified, | ||
| dependencies, | ||
| status, | ||
| }: { | ||
| componentsModified: number; | ||
| dependencies: number; | ||
| status: string; | ||
| }) => ( | ||
| <Box flexDirection="column"> | ||
| <Box> | ||
| <Box width={30}> | ||
| <Text>Files modified:</Text> | ||
| </Box> | ||
| <Box justifyContent="flex-end" width={3}> | ||
| {componentsModified} | ||
| </Box> | ||
| </Box> | ||
| <Box> | ||
| <Box width={30}> | ||
| <Text>Files potentially affected:</Text> | ||
| </Box> | ||
| <Box justifyContent="flex-end" width={3}> | ||
| {status === 'loading' ? '⏳' : dependencies} | ||
| </Box> | ||
| </Box> | ||
| </Box> | ||
| ); | ||
|
|
||
| const App = () => { | ||
| const [stagedFiles, setStagedFiles] = useState([]); | ||
| const [data, setData] = useState([]); | ||
| const [dataStatus, setDataStatus] = useState('loading'); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it makes more sense to make this a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right now it's loading/loaded, but there can be other loading states, such as 'error' or 'cancelled' that I'd like to leave this open for.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In that case can we have the vale be an enum rather than an arbitary string. There's an example of that in #2067 enum Status {
Pending = 'PENDING',
Loaded = 'LOADED',
Errored = 'ERRORED',
}Then in your useState: const [status, setStatus] = useState<Status>(Status.Pending); |
||
|
|
||
| useEffect(() => { | ||
| const getStagedFiles = async () => { | ||
| const staged = (await getGitStagedFiles('src/')) as string[]; | ||
| setStagedFiles(staged); | ||
|
|
||
| if (staged.length === 0) { | ||
| setDataStatus('loaded'); | ||
| } | ||
| }; | ||
| getStagedFiles(); | ||
| }, []); | ||
|
|
||
| useEffect(() => { | ||
| if (stagedFiles.length > 0) { | ||
| const dependencies = getDependencies( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not too happy with the API here, what do you think? Any suggestions for how to improve it?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also think we can improve it. Since we'll gain insights as you explore how to make this work for GitHub, webpack, and storybook… I'd like to wait to revamp it when you know more about all these other use cases. |
||
| 'src/**/*.tsx', | ||
| '*.test.tsx', | ||
| stagedFiles, | ||
| ); | ||
| setData(formatDependencies(dependencies)); | ||
| setDataStatus('loaded'); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You never set this prop to
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure what you mean by that, but let's talk about it during our next pairing session |
||
| } | ||
| }, [setData, stagedFiles]); | ||
|
|
||
| return ( | ||
| <React.Fragment> | ||
| <Box marginBottom={1} flexDirection="column"> | ||
| <Box> | ||
| <Box width={3}>💦</Box> | ||
| <Box> | ||
| <Text bold>yarn splash</Text>: Observe the splash zone of a change | ||
| across the entire library | ||
| </Box> | ||
| </Box> | ||
| </Box> | ||
| <Components components={data} status={dataStatus} /> | ||
| <Summary | ||
| componentsModified={stagedFiles.length} | ||
| status={dataStatus} | ||
| dependencies={ | ||
| new Map([ | ||
| ...data.map(({pathname, filename}) => [ | ||
| pathname, | ||
| pathname + filename, | ||
| ]), | ||
| ...data.reduce( | ||
| (val, curr) => | ||
| val.concat( | ||
| curr.dependencies.map(({pathname}) => [ | ||
| pathname, | ||
| curr.pathname + curr.filename, | ||
| ]), | ||
| ), | ||
| [], | ||
| ), | ||
| ]).size | ||
| } | ||
| /> | ||
| <Box marginTop={1}> | ||
| <Color dim> | ||
| <Box width={3}>💡</Box> | ||
| <Box> | ||
| Tip: command + click a file path to open it in your text editor | ||
| </Box> | ||
| </Color> | ||
| </Box> | ||
| </React.Fragment> | ||
| ); | ||
| }; | ||
|
|
||
| render(<App />); | ||
Uh oh!
There was an error while loading. Please reload this page.