arc icon indicating copy to clipboard operation
arc copied to clipboard

require.context

Open diegohaz opened this issue 7 years ago • 20 comments

I'm creating this issue to unify the discussions about require.context.

While that's very handy to import components, it comes with some drawbacks like #130, #113 and https://github.com/diegohaz/arc/issues/128#issuecomment-281914665

Personally, I don't have problems with it, the advantages are bigger than the issues for me. But, as many are having problems, we should consider removing it by default or at least give proper instructions on how to remove it.

If you are using this webpack feature (whether your project is based on ARc or not), please share your thoughts.

diegohaz avatar Feb 24 '17 22:02 diegohaz

I love the feature of just doing import { XYZ } from 'components'. (I just got caught doing some stuff "prematurely" 😁 .)

I'm not in favor of removing it by default.

zentuit avatar Feb 24 '17 22:02 zentuit

Initially I did love the opportunity to import modules from 'containers' or 'components' but now I came across an issue which I don't seem to find an easy solution to.

I have a LoginPage container that map some props to the LoginPage (page) component which in turn render (among other things) a <LoginForm /> which is basically another container decorated with redux-form.

If in my LoginPage component I import the LoginForm container like this import { LoginForm } from 'containers' it doesn't work, because basically by the time this is imported in the LoginPage container which decorate the LoginPage component which requires the LoginForm container this container has not been exported yet. If I instead import it directly from the path the file is in, like this import LoginForm from '../../../containers/LoginForm.js' it works correctly.

Sorry fo the headache, I hope the above scenario somehow make sense. I couldn't figure out a way to get around this reference issue with using the import from 'container'; unless I'm missing something here I believe this could create serious limitations.

santino avatar Feb 28 '17 20:02 santino

@santino : that sounds similar to what happened to me. Basically, you have to use the components you import in function so they are defined when the function is called. See #130. By moving the use of the imported component into a function, everything works.

zentuit avatar Feb 28 '17 22:02 zentuit

Node resolves those circular dependencies at runtime. Just remember to wrap the components. These are the rules:

  • If you are importing a container or a component from the same atomic design level, just remember to wrap it

    // components/organisms/SomeOrganism
    import { Organism } from 'components'
    
    // bad
    const StyledOrganism = styled(Organism)``
    
    // good
    const StyledOrganism = styled(props => <Organism {...props} />)``
    
    // good - rendering the component is equivalent to wrapping it
    const SomeOrganism = () => (
      <div>
        <Organism />
        ...
      </div>
    )
    
  • On containers, always wrap the components

    // containers/OrganismContainer.js
    import { Organism } from 'components'
    
    // bad
    export default connect(...)(Organism)
    
    // good
    const OrganismContainer = props => <Organism {...props} />
    export default connect(...)(OrganismContainer)
    

Following those rules should be enough. We should add it to the new Wiki. Please, tell me if there's another use case.

diegohaz avatar Mar 01 '17 09:03 diegohaz

Thanks @zentuit and @diegohaz for your suggestions; eventually wrapping the component on the container solved the issue.

This feels like a work around but not a big compromise to live with so, going back to the original topic, I agree that the benefits are more than the disadvantages hence I vote against removing it by default. There is certainly need for more documentation around this as I can see people spending time to get to the source of the issue.

I also take the opportunity of this comment to thank you @diegohaz for sharing such a good work, your boilerplate is exactly what I was looking for.

santino avatar Mar 01 '17 19:03 santino

Not sure how nicely require.context plays with tree-shaking, an important feature of Webpack.

Also, I personally love WebStorm's automatic imports (Alt + Enter or Cmd + Enter automatically adds the correct, most exact, import at the top). This is simply not applicable with require.context.

I've had issues running this code in SSR using universal-webpack. (Yes, I've basically gone and made a brand new boilerplate based on the PR I submitted to arc a few weeks ago... with support for toggling SSR on and off, and much easier SSR support for various libraries.) Not sure if this is relevant here, as webpack-isometric-tools are used instead.

Finally, I've had issues with this when trying to run the server for SSR without bundling. I have a development server that uses babel-require and other similar tools and allows a much easier, faster debugging experience for me. But, since there's no webpack involved, require.context doesn't work.

advance512 avatar Mar 08 '17 17:03 advance512

Maybe we can add a npm script to automatically add files to index.js to be ran concurrently with npm start/npm run dev in watch mode?

diegohaz avatar Mar 09 '17 13:03 diegohaz

https://github.com/gajus/create-index

This might be useful?

advance512 avatar Mar 14 '17 12:03 advance512

@advance512 Am I correct in assuming it's an automated way of doing this below?

https://github.com/diegohaz/arc/blob/0b9cacb2767a77ac4ea55f0a8c8c5b6c757ebac0/src/components/index.js

Could be useful in making generating the above file automated and easy, but maybe it would create some confusion for new users who just want to understand how to add/import/export new components without having to read the README.

declanelcocks avatar Mar 20 '17 04:03 declanelcocks

@declanelcocks yes, of a sorts.

The main point is not confusing WebStorm's new feature: auto import of React components. https://www.jetbrains.com/webstorm/whatsnew/

This is also relevant for actions, and in my case for GraphQL queries.

advance512 avatar Mar 21 '17 03:03 advance512

@advance512 Out of curiosity did you try to implement something like the above?

declanelcocks avatar Mar 21 '17 05:03 declanelcocks

No, I actually completely discarded this method. I also stopped exporting as default, rather I export by the identifier (e.g. control, class, whatever) name. It makes auto-imports almost as good as in Python.. and I don't care about the paths as I never manually write them.

advance512 avatar Mar 21 '17 05:03 advance512

I was thinking about how would look a generic CLI to completely replace our usage of require.context. While I was writing I realized that it was becoming overkill. Well, I did it for everything except sagas. I'm leaving it here in case of someone wants to create something like that.

Generate components index (--export-folder)
$ generate-index "src/components/*/*" --output "src/components/index.js" --export-folder --watch

Output:

export Atom from './atoms/Atom'
export Button from './atoms/Button'
...

So we can use it as we do today:

import { Atom, Button } from 'components'
Generate actions index (--export-all)
$ generate-index "src/store/*/actions.js" --output "src/store/actions.js" --export-all --watch

Output:

// is this possible?
export * from './modal/actions'
export * from './post/actions'
...

So we can use it as we do today:

import { modalShow } from 'store/actions'
Generate middleware index (--export-default-array)
$ generate-index "src/store/*/middleware.js" --output "src/store/middlewares.js" --export-default-array --watch

Output:

import entities from './entities/middleware'
...
export default [
  entities,
  ...
]

So we can use it as we do today:

import middlewares from 'store/middlewares'

const configureStore = initialState =>
  createStore(reducer, initialState, applyMiddleware(...middlewares))
Generate reducers index (--export-default-object)
$ generate-index "src/store/*/reducer.js" --output "src/store/reducers.js" --export-default-object --watch

Output:

import entities from './entities/reducer'
import modal from './modal/reducer'
import post from './post/reducer'
...
export default {
  entities,
  modal,
  post,
  ...
}

So we can use:

import { combineReducers } from 'redux'
import { routerReducer as routing } from 'react-router-redux'
import reducers from 'store/reducers'

export default combineReducers({
  routing,
  ...reducers,
})
Generate selectors index (--export-all-named)
$ generate-index "src/store/*/selectors.js" --output "src/store/selectors.js" --export-all-named --watch

Output:

export * as entities from './entities/selectors'
export * as modal from './modal/selectors'
export * as post from './post/selectors'
...

So we can use:

import upperFirst from 'lodash/upperFirst'
import * as modules from 'store/selectors'

export default Object.keys(modules).reduce((rootSelectors, moduleName) => {
  const fromName = `from${upperFirst(moduleName)}`
  const moduleSelectors = modules[moduleName]
  const getState = (state = {}) => state[moduleName] || {}

  rootSelectors[fromName] = Object.keys(moduleSelectors)
    .filter(key => typeof moduleSelectors[key] === 'function')
    .reduce((object, key) => {
      const selector = moduleSelectors[key]
      object[key] = (state, ...args) => selector(getState(state), ...args)
      return object
    }, {})

  return rootSelectors
}, {})

And import it as we do today:

import { fromEntities, fromPost } from 'store/selectors'

fromEntities.getDetail(state, fromPost.getDetail())

diegohaz avatar Mar 22 '17 02:03 diegohaz

@diegohaz You're using a library for this generate-index? Just asking as I couldn't find it

declanelcocks avatar Mar 22 '17 03:03 declanelcocks

@declanelcocks No, that's just an interface idea for a possible library.

diegohaz avatar Mar 22 '17 03:03 diegohaz

Ah ok, makes sense. I suppose some of the code from the current index.js file could be re-used to create the logic for generating all the import statements.

declanelcocks avatar Mar 22 '17 03:03 declanelcocks

@diegohaz Also noticed that even doing export Atom from './atoms/Atom' (actually had to use { default as Atom } for every component in an index.js file, I will still end up with one big chunk with the entire app code.

declanelcocks avatar Mar 22 '17 05:03 declanelcocks

What I ended up doing was this for my atoms, molecules and organisms as they never (in my case) clash:

const req = require.context('.', true, /\.\/(atoms|molecules|organisms)\/[^\/]+\/index\.js/);

req.keys().forEach((key) => {
	const componentName = key.replace(/^.+\/([^/]+)\/index\.js/, '$1');
	module.exports[componentName] = req(key).default;
});

And for my containers I just added containers.js to my components directory which does the following. Same goes for pages and templates:

const req = require.context('.', true, /\.\/containers\/[^\/]+\/index\.js/);

req.keys().forEach((key) => {
	const componentName = key.replace(/^.+\/([^/]+)\/index\.js/, '$1');
	module.exports[componentName] = req(key).default;
});

This way you can do:

import { Atom, Molecule, Organism } from 'components';
import { Container } from 'components/container';
import { Page } from 'components/pages';

nealoke avatar May 20 '17 14:05 nealoke

How is this working with jest ? I get this error when I try to run my tests: TypeError: require.context is not a function

ghalex avatar Jul 11 '17 15:07 ghalex

@ghalex require.context doesn't work with jest. That's one of the reasons we mock all components except the one being tested. https://github.com/diegohaz/arc/blob/master/private/jest/componentsMock.js

diegohaz avatar Jul 11 '17 16:07 diegohaz