astroturfcss / astroturf

Better Styling through Compiling: CSS-in-JS for those that want it all.
https://astroturfcss.github.io/astroturf/
MIT License
2.28k stars 60 forks source link

0.8: Cannot read property 'node' of null #64

Closed ai closed 5 years ago

ai commented 6 years ago

I updated astroturf and got this error during test:

    TypeError: Cannot read property 'node' of null

      at importNodes.forEach.path (node_modules/astroturf/plugin.js:180:18)
          at Array.forEach (<anonymous>)
      at PluginPass.post (node_modules/astroturf/plugin.js:175:19)
      at transformFile (node_modules/@babel/core/lib/transformation/index.js:94:27)
      at runSync (node_modules/@babel/core/lib/transformation/index.js:45:3)
      at transformSync (node_modules/@babel/core/lib/transform.js:43:38)
      at transform (node_modules/@babel/core/lib/transform.js:22:38)

My Babel config:

{
  "presets": [
    [
      "@babel/preset-env",
      {
        "loose": true
      }
    ]
  ],
  "plugins": [
    "./builder/auto-import",
    "babel-plugin-transform-typograf",
    [
      "@babel/plugin-proposal-decorators",
      {
        "legacy": true
      }
    ],
    "@babel/plugin-syntax-dynamic-import",
    "@babel/plugin-proposal-class-properties",
    "@babel/plugin-proposal-object-rest-spread"
  ],
  "env": {
    "test": {
      "plugins": [
        "babel-plugin-dynamic-import-node",
        [
          "astroturf/plugin",
          {
            "allowGlobal": true,
            "writeFiles": false
          }
        ],
        [
          "babel-plugin-transform-rename-import",
          {
            "replacements": [
              {
                "original": "^.*\\.(css|less|sss)$",
                "replacement": "identity-obj-proxy"
              }
            ]
          }
        ]
      ]
    }
  }
}
jquense commented 6 years ago

Is it failing on a particular piece of code? Sounds like the import triing is missing an edge case perhaps

ai commented 6 years ago

I see this error during run of any test, so I think it is not related with some specific code:

For most of the cases we have code like:

// Import will be added by ./builder/auto-import

let Container = styled.div`
  position: relative;
  font-size: 13px;
`
ai commented 6 years ago

I think error can be the result of astroturf plugin options:

            "allowGlobal": true,
            "writeFiles": false
vrizo commented 5 years ago

Unfortunately, it still doesn’t work. Tried to update to 0.8.2 and almost all the tests fail. There is no such error when a test doesn't contain styles, e.g., reducers or pure functions.

I tried to add

if (!decl) return

to node_modules/astroturf/plugin.js:182 — it works perfectly.

@jquense can I help you somehow?

jquense commented 5 years ago

@vrizo happy to take a PR we aren't hitting this error and i'm bit short on time to debug. My guess is that the import trimming logic probably assumes that the imports exist, conflict with the global option, probably just needs a guard somewhere as you noted

vrizo commented 5 years ago

Yep, looks like you're right. But I think if (!decl) return hides a true cause, because I can't replicate the problem in your tests. Moreover, our test file includes components with astroturf imports. I'm going to debug a little bit more today

vrizo commented 5 years ago

@jquense Hi! Sorry for the delay, too many bugs this week. I'm still trying to investigate the problem.

First of all, we have an auto import of styled if parser finds styled. in a code. But I need to import { css } manually.

I tried to reproduce the issue in your tests but with no luck.

So I go further and debug the file with import { css }:

      // plugin.js L173
      const importNodes = file.get(IMPORTS);
      const imports = [];
      importNodes.forEach(path => {
        console.log('DEBUG: ', path.type, path.isImportDeclaration(), t.isImportDeclaration(path))

        const decl = !path.isImportDeclaration() ? path.findParent(p => p.isImportDeclaration()) : path;

Result in console:

DEBUG:  ImportDeclaration false true

In this case, path.type is ImportDeclaration, but it has no path.node. I have minimal experience in Babel, and I can't find why path doesn't include node. Is it correct behavior?

hallister commented 5 years ago

Ran into the exact same problem, with the exact same solution: bailing when !decl node_modules/astroturf/plugin.js:182 works normally. Same problem occurred for the webpack config for Storybook and 2 other packages.

So there are clearly cases where path doesn't have a node property but it's unclear what those cases are.

ai commented 5 years ago

@jquense I found a reason and sent fix https://github.com/4Catalyzer/astroturf/pull/137