Thanks for the details, @cdfa. I was able to reproduce this error during the SSR. Will update with more technical details in the nearest future.

from atomic-layout.

Update: I'm planning to merge the pull request that will resolve the mentioned issue in the nearest future. The implementation is done, tests are passing, and there is only a few things left to check. Thank you for patience.

from atomic-layout.

I created a branch using dummy data: https://github.com/svsticky/static-sticky/tree/dummy-data.
Installation instruction are in the README.

If you think it's not isolated enough, I can do more, but I think this should be okay for now.
I hope the problem is easily found and solved.

from atomic-layout.

Updating to 0.7.1 seems to have fixed it! Thank you so much for your help.

from atomic-layout.

In a nutshell, SSR logic of Atomic layout consists of SSR of:

• styled-components (present)
• react-responsive (present)

It may also need some additional api (see SSR of react-responsive)

from atomic-layout.

SSR is also a crucial point when thinking about pluggable css-in-js solution, as different ones have different server-side styles flushing API and techniques.

from atomic-layout.

I've added a simple unit test that attempts to render an application to string via ReactDOMServer.renderToString(), when application renders all the components from atomic-layout. There is no result string assertion, since it's somewhat pointless. As I've mentioned, Atomic layout doesn't perform server-specific logic during rendering (no class names flashing, etc.), thus its string output would depend on the output from styled-components and react-responsive.

from atomic-layout.

I ran the following error while trying to use atomic-layout in my app:

WebpackError: TypeError: Cannot read property '1' of null
- index.js:113 
[lib]/[0.6.2]/[atomic-layout]/lib/index.js:113:8165
- index.js:113 
[lib]/[0.6.2]/[atomic-layout]/lib/index.js:113:8027
- index.js:113 o
[lib]/[0.6.2]/[atomic-layout]/lib/index.js:113:7933
- index.js:113 r
[lib]/[0.6.2]/[atomic-layout]/lib/index.js:113:7227
- index.js:113 new r
[lib]/[0.6.2]/[atomic-layout]/lib/index.js:113:6968
- index.js:113 o
[lib]/[0.6.2]/[atomic-layout]/lib/index.js:113:7080
- index.js:113 S
[lib]/[0.6.2]/[atomic-layout]/lib/index.js:113:4571
- index.js:113 value
[lib]/[0.6.2]/[atomic-layout]/lib/index.js:113:5826
- bootstrap:25 a.render
lib/webpack/bootstrap:25:1
- bootstrap:24 a.read
lib/webpack/bootstrap:24:1
- bootstrap:36 renderToString
lib/webpack/bootstrap:36:1
- static-entry.js:190 Module../.cache/static-entry.js.__webpack_exports__.default
lib/.cache/static-entry.js:190:18

This only happens when using SSR, not in a development build. This seems to be a limitation of react-responsive, but there are several packages that supposedly resolve the issue (react-responsive-redux), but they expose their own MediaQuery component, which doesn't get used in atomic-layout. Do you have any idea how to resolve this?

from atomic-layout.

Hello, @cdfa.

Could you please provide the following info, so we could debug it better:

• The versions of react and styled-components installed in your application
• The component source (the excerpt it enough) that renders the components from the atomic-layout package
• The excerpt (server code) of how your SSR is performed

Thanks.

from atomic-layout.

Of course :)
We're using react 16.7.0 and styled-components 4.1.3.

We use Gatsby's default SSR, which can be found at https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby/cache-dir/static-entry.js

Component:

<Composition
  template={`
    logo
    info
    members
  `}
  templateSm={
    /*I couldn't find a better way to remove the spacing between the logo and members when the info is higher than their combined height, because their rows will get stretched.*/ `
    logoAboveMembers info
  `
  }
  templateMd={`
    logo info info members
  `}
  gutter="10px"
  marginTop={'10px' /* spacing for the button */}
>
  {({ Logo, Info, Members, LogoAboveMembers }) => (
    <>
      <Logo>
        <Sticky>{renderLogo(committee)}</Sticky>
      </Logo>
      <Info>
        <Header className="huge">{committee.name}</Header>
        <Markdown>{committee.description.description}</Markdown>
      </Info>
      <Members>
        <Sticky>{renderMembers(committee)}</Sticky>
      </Members>
      <LogoAboveMembers>
        <Sticky>
          {renderLogo(committee)}
          {renderMembers(committee)}
        </Sticky>
      </LogoAboveMembers>
    </>
  )}
</Composition>

from atomic-layout.

The reason

The issue is caused by the invalid regular expression from the css-mediaquery package:

https://github.com/ericf/css-mediaquery/blob/ad119660550806964fc91170cc7da83ac59c513b/index.js#L15

This regular expression is being applied to the media query expression, which doesn't match in case of the calc usage to compute a media query's value:

'(max-width: calc(250px - 1px))'.match(/^\(\s*([_a-z-][_a-z0-9-]*)\s*(?:\:\s*([^\)]+))?\s*\)$/)
// null

Considering the package is 5 years old, maybe the calc wasn't even in the spec back then.

Nevertheless, the issue tree is the following:

atomic-layout
  react-responsive
    matchmediaquery
      css-mediaquery

The last two packages are extremely outdated, and produce unexpected behavior when used with the modern features of CSS.

The solution

A proper RegExp that would take calc expressions into account can look as follows:

\(\s*([^\s\:\)]+)\s*(?:\:\s*(.+))\)

Using it instead of the invalid one by modifying the source of the problematic package resolves the issue, and results into no exceptions during SSR. However, the package that deep in the dependency tree cannot be easily monkey-patched. This makes the possible ways to solve:

• Creating a pull request to css-mediaquery, requesting its version bump, requesting matchmediaquery version bump, requesting react-responsive version bump
• Replacing css-mediaquery during the build of atomic-layout with the local fixed fork
• Forking css-mediaquery and publishing a modern fixed version
• Creating a custom <MediaQuery> component, utilizing latest browser API and React Hooks, getting rid of all that dependency tree at once

I need to think which would work the best for the library and its future support cycle.

from atomic-layout.

Wow, I had no idea it would go that deep. I think the fastest way to fix the issue is replacing css-media-query with a local fixed fork.
Then I suggest creating the PR if it's not too much work. If there's no response, I'd contact the react-responsive author to collaborate on a modern <MediaQuery> component, since that library seems to be relatively well maintained as well and has some standalone use.

from atomic-layout.

I will check what would be needed to use a local fork as the replacement. However, I am not a fan of this idea, as it effectively creates a separate package to maintain, while retaining all the possible issues from the origin.

Instead, I've started a rewrite of the internal logic in #115, and it's already proven to be almost everything needed. I hope we will release the dependency-free, smaller version of Atomic layout in the future.

from atomic-layout.

I don't have much experience with local packages either, but I think you can clone the repo, make the modifications (like replacing the regex) and use npm link.

Of course I don't know much about the internals of this package, but if you're making something to replace the <MediaQuery> component I suggest looking at if (a part of) it could be used by react-responsive as well and notifying it's developer. Would be a small effort to help them along a fair bit.

Anyway those are my thoughts. A smaller package is great too and I'm happy this issue is developing so quickly!

from atomic-layout.

@cdfa Could you please try atomic-layout@beta, which uses its own MediaQuery component and doesn't contain the mentioned issue? Server-side rendering should be fine now. Please let me know in this thread in case there's any issue. Thanks.

from atomic-layout.

It does seem to work for the most part, but in the excerpt i gave above, the templateMd only seems to work after I change to a smaller display size, switching to a different template and then back again. On initial render, <Logo> and <Info> seem to overlap and <Members> takes more than half of the available width.

from atomic-layout.

Okay, at least that doesn't sound like a calculation issue anymore.

Do you have a chance to set up a small demo somewhere, please? This would help to investigate.

from atomic-layout.

the repo is open source, but our data comes is sourced from Can for which you need an API key. I'll look into setting up a repo using local data instead.

from atomic-layout.

@cdfa I believe there is no need to expose any sensitive data, or API keys. Yes, local "dummy" data repo sounds like a great idea. I'm only interested in two things:

1. Project setup (SSR, build, etc.)
2. Usage point of Atomic layout.

Once I can debug those, I can conclude what's going wrong.

from atomic-layout.

I wasn't planning on giving you our API key 😉, just making a fork with some fake data. I expect the build will fail when the data from Contentful isn't available.
Anyway, we're using Gatsby for SSR and building and we're currently trying to use Atomic layout in here.

I'm not sure if that's enough to help you debug, but otherwise I'll try to have a fake data fork soon.

from atomic-layout.

Hi, @cdfa. Getting back to you with updates.

I've cloned and run your repository locally, but couldn't spot any issues in src/templates/CommitteeTemplate.jsx. I've also tried running your app without JavaScript to test SSR behavior, but it doesn't seem it can run server-side. At least, I wasn't successful in trying to reproduce that.

Please, could you try updating to [email protected] (the latest version), and verifying the issue is still there? I may need yourt assistance to understand better what is going off. If the issue persist, may I kindly ask you to report it as a sepaerate GitHub issue in this repo? Thank you.

from atomic-layout.

Glad to help, @cdfa! Please let me know in case you experience any other issues. That includes if you feel that something is not clear, or you cannot find something in the documentation.

from atomic-layout.