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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] move components to directories #3212

Closed
wants to merge 1 commit into from
Closed

[WIP] move components to directories #3212

wants to merge 1 commit into from

Conversation

mbrookes
Copy link
Member

@mbrookes mbrookes commented Feb 7, 2016

I've been experimenting with putting the component files that are currently in the top level src directory into their own subdirectories, and have run into a an interesting problem.

When (ignoring case) the directory name is the same as the legacy filename, that is to say, for files with a single word name (i.e. those that don't have a hyphen in them), the import fails if you specify the directory to import from:

import Avatar from 'material-ui/lib/Avatar'

instead, you have to specify the full path:

import Avatar from 'material-ui/lib/Avatar/Avatar'

or:

import Avatar from 'material-ui/lib/Avatar/index'

Even then it complains about duplicate paths - but at least it loads.

Unless there's a clever workaround (or I'm misunderstanding the problem), this is going to be a bit of a hitch for standardising the directory layout and simplifying imports, while maintaining backwards compatibility with the current file-name (with a deprecation warning inserted).

@newoga
Copy link
Contributor

newoga commented Feb 7, 2016

Yeah this is going to be a pain 😆

Back in December, @alitaheri reminded me and @oliviertassinari about this issue (in that call you helped orchestrate actually 😆). Apparently our only option is to do the migration all at once. We could create a codemod like at https://github.com/reactjs/react-codemod to help assist developers with the migration in their own codebases.

Like you said, components with composite names like dropdown-menu.jsx, and text-field.jsx can maintain backwards compatibility because of the hyphen. Though I didn't realize that specifying the full path makes it load with a warning. That's good to know.

@alitaheri
Copy link
Member

Or! when we publish, we flatted the requires like:

node_modules/material-ui/Avatar
node_modules/material-ui/AppBar
node_modules/material-ui/Divider
node_modules/material-ui/FlatButton
etc...

import Avatar from 'material-ui/Avatar';

while keeping a lib folder with compatibility files like:

node_modules/material-ui/lib/avatar
node_modules/material-ui/app-bar

import Avatar from 'material-ui/lib/avatar';

lib/avatar will be like:

import Avatar from '../Avatar';

export default Avatar;

You see this pattern in many places. npm for example has branches for these kinds of thing!

This is gonna ease the pain dramatically 😁

@newoga
Copy link
Contributor

newoga commented Feb 7, 2016

Or! when we publish, we flatted the requires like:
import Avatar from 'material-ui/Avatar';

I really like that import path 😆

@mbrookes
Copy link
Member Author

mbrookes commented Feb 7, 2016

Or! when we publish, we flatted the requires like:
import Avatar from 'material-ui/Avatar';

I really like that import path 😆

Same. 😄

How is that achieved in practice? Do we need to maintain a build script with source and destination paths?

What does the source tree look like in the ideal scenario?

At the moment we have some as component-file.jsx in src, some in component-sub-directory, some in ComponentSubDirectory with ComponentName.jsx and index.js(x), and a mix of public and internal subcomponents.

@alitaheri
Copy link
Member

@mbrookes Yeah, we will have to maintain a map too. like one for the docs site. but since components aren't added that often it's not a big overhead in my opinion.

@mbrookes mbrookes added the on hold There is a blocker, we need to wait label Mar 2, 2016
@mbrookes mbrookes self-assigned this Mar 2, 2016
@mbrookes mbrookes added the Core label Mar 5, 2016
@mbrookes
Copy link
Member Author

mbrookes commented Mar 9, 2016

Closing in favour of discussion here #2679. Please copy over any relevant comments not reflected there.

@mbrookes mbrookes closed this Mar 9, 2016
@oliviertassinari oliviertassinari removed the on hold There is a blocker, we need to wait label Mar 19, 2016
@mbrookes mbrookes deleted the rename-components branch March 19, 2016 01:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants