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

Import order situation/strategies #130

Closed
zentuit opened this issue Feb 24, 2017 · 10 comments
Closed

Import order situation/strategies #130

zentuit opened this issue Feb 24, 2017 · 10 comments
Labels

Comments

@zentuit
Copy link

zentuit commented Feb 24, 2017

I've run into a situation where I have a component at trying to import another component at its same "level" (ie Molecule). If the name of the component that I'm importing falls before the importer component in the sort order, then everything imports correctly. However, if the name comes after, then the component imported is undefined.

Ex:
|- atoms
|- molecules
| |- AComponent
| |- BComponent
| |- CComponent

If BComponent has import AComponent from 'components' then everything is fine; AComponent is defined.

But if BComponent has import CComponent from 'components' then there is a problem; CComponent is undefined.

One way to solve that is to move BComponent up to Organism; but sometimes that could cause a chain reaction.

Another way is to require CComponent when needed if possible -- that's what I'm doing.

Are there any other strategies to deal with this situation?

@diegohaz
Copy link
Owner

Could you share the piece of code where you are trying to use the undefined component?

Most cases are solved this way:

import styled from 'styled-components'
import { CComponent } from 'components'

// fails
const StyledCComponent = styled(CComponent)``

// good
const StyledCComponent = styled(props => <CComponent {...props} />)``

@zentuit
Copy link
Author

zentuit commented Feb 24, 2017

If I put console.log(CComponent) right after that import you have there, then it will report as undefined.

@diegohaz
Copy link
Owner

diegohaz commented Feb 24, 2017

Yeah, that's normal. This is a circular dependency and Node should be able to resolve this at runtime, thus wrapping the component.

Can you try the code from my last comment?

@zentuit
Copy link
Author

zentuit commented Feb 24, 2017

Ok I see the issue. I need to load the imported components into a lookup table so I can render the correct component based on the list of entries passed to my component.

I get a list of entries from the database, a mixture of articles, slides, etc. I use a meta data field to determine which component should be used.

Doing this:

import { Article, Testimonial } from 'components'

const lookup = {
  article: Article,
  testimony: Slide,
}

const Carousel = ({ items, ...props}) => {
    const slides = items.map((key, idx) => {
        const Comp = lookup[slides[key].meta.type]
        return <Comp key={idx} {...props} />
    }
<snip>
}

would result in Testimonials being undefined and so I would get a React error.

Moving the lookup creation into the Carousel component let everything resolve.

@diegohaz
Copy link
Owner

Nice. Thank you for the feedback. :)

@keego
Copy link
Contributor

keego commented Sep 9, 2017

I love the dynamic importing done with components in arc and have been using it in my latest Vue.js project. I've hit this circular component dependency issue before and finally managed to find a solution that works. I first iterate over all the components and export an empty object. Then I reiterate over all the components and actually start requiring them, merging each module into module.exports. I got the idea from how webpack manages circular dependencies. Here's the code I'm using:

const merge = require('lodash/merge')

const req = require.context('.', true, /\.\/[^/]+\/[^/]+\.vue$/)

// This exports all components, resolving circular dependencies in two steps:
// 1. export empty objects in place of each component
// 2. require each component and merge it into the existing export for that module

const componentPaths = []

// export an empty object for each component
req.keys().forEach((key) => {
  const componentName = key.replace(/^.+\/([^/]+)\.vue/, '$1')
  componentPaths.push({ componentName, key })
  module.exports[componentName] = {}
})

// actually start exporting each component
componentPaths.forEach(({ componentName, key }) => {
  merge(module.exports[componentName], req(key).default)
})

@diegohaz
Copy link
Owner

diegohaz commented Sep 9, 2017

@keego Wow!

Do you know if that works with styled-components when doing styled(CircularDependentComponent)?

Also, I wonder if is that _.merge really necessary. If so, it would be better to require('lodash/merge') instead of requiring the entire package (it increases the bundle size).

Anyway, it would be an amazing PR if you want to do that.

@keego
Copy link
Contributor

keego commented Sep 11, 2017

@diegohaz Thanks!

Unfortunately, I just verified it does not work with styled(CircularDependentComponent). I'm not too surprised though as I've never seen circular dependencies get properly resolved without delayed evaluation. I think the issue there is still with the immediate invocation and, although it doesn't feel the most satisfying, I agree with your comment above about using styled(props => <CComponent {...props} />) instead.

Also you're very much right about including 'lodash/merge' instead of 'lodash', I just use lodash so much in my project that I've started requiring the whole thing. I just updated my comment :)

This mainly solves the specific issue described in the original post, and solves the circular dependency case for things that do allow for delayed evaluation (i.e. defining circular components in my vue project, and with certain cases of defining circular components in the arc project)

I can put this into a PR this week 👍

@lastmaj
Copy link

lastmaj commented Mar 15, 2022

following the comment i went from
export const LogoWrapper = styled(ReactSVG)
to
export const LogoWrapper = styled( props => <ReactSVG {...props} />)

I get the following error :
Unexpected token, expected ","

any help ?

@ezgitetik
Copy link

ezgitetik commented Sep 27, 2022

@lastmaj you should convert file extension to .jsx (or .tsx) and import React. otherwise it does not recognize react component.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants