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

Next 6 breaks styled-jsx with sass plugin #4265

Closed
jakegibson opened this issue May 3, 2018 · 22 comments
Closed

Next 6 breaks styled-jsx with sass plugin #4265

jakegibson opened this issue May 3, 2018 · 22 comments

Comments

@jakegibson
Copy link

jakegibson commented May 3, 2018

  • [X ] I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

Should be able to upgrade next 6 and still use styled-jsx with sass

Current Behavior

Working with next 5.1.0 upgrading to next 6 breaks importing a scss file to be used in styled-jsx
example

import style from './componentName.scss';

in component: <styled jsx>{style}</style>

On next 6 I get 'style is undefined'

package.json
"extract-text-webpack-plugin": "^3.0.2",
"extracted-loader": "^1.0.3",
"node-sass": "^4.7.2",
"postcss": "^6.0.16",
"postcss-cssnext": "^3.0.2",
"postcss-loader": "^2.0.10",
"sass-loader": "^6.0.6",
"styled-jsx-css-loader": "^0.3.0"
"styled-jsx-plugin-sass": "^0.2.2",
"autoprefixer": "^8.2.0",

babelrc

  "presets": [
    [
      "next/babel",
      {
       "preset-env": { "modules": "commonjs" }
      }
    ]
  ],
  "plugins": [
    [
      "styled-jsx/babel",
      { "plugins": [
          ["styled-jsx-plugin-sass", {
              "sassOptions": {
                "includePaths": ["./styles"],
                "precision": 2
              }
            }
          ]
        ]
      }
    ]
  ]
}
```

next.config.js
```const ExtractTextPlugin = require('extract-text-webpack-plugin');

const dev = process.env.NODE_ENV !== 'production';
const path = require('path');
const getRoutes = require('./routes');

module.exports = {
  exportPathMap: getRoutes,
  webpack: (config) => {
    config.module.rules.push({
      test: /\.scss$/,
      use: [
        {
          loader: 'emit-file-loader',
          options: {
            name: 'dist/[path][name].[ext]',
          },
        },
        'babel-loader',
        'styled-jsx-css-loader',
        'postcss-loader',
      ],
    });
    return config;
  },
};```
## Steps to Reproduce (for bugs)
<!--- Provide a link to a live example, or an unambiguous set of steps to -->
<!--- reproduce this bug. Include code to reproduce, if relevant -->
1.
2.
3.
4.

## Context
<!--- How has this issue affected you? What are you trying to accomplish? -->
<!--- Providing context helps us come up with a solution that is most useful in the real world -->

## Your Environment
<!--- Include as many relevant details about the environment you experienced the bug in -->
| Tech    | Version |
|---------|---------|
| next    |    6     |
| node    |      8.10.0   |
| OS      |    mac and windows     |
| browser |         |
| etc     |         |
@rashidul0405
Copy link
Contributor

You may check this package: https://github.com/zeit/next-plugins/tree/master/packages/next-sass

@jakegibson
Copy link
Author

jakegibson commented May 4, 2018

I saw that, but prefer the automatic name spacing that styled-jsx provides without having to import every classname with css modules. As well as not having to update 50+ pages/components
className="myClass" with styled-jsx vs import style from './style.css className={style.className}

@rschlaefli
Copy link

rschlaefli commented May 4, 2018

Edit: Seems that this was in our case caused by another babel plugin (specifically transform-react-constant-elements, vercel/styled-jsx#85).

This seems to affect us too (but a little differently): we @import 'src/theme.scss; an scss file with theme settings in most of our components and, only after deploying the application, the layout completely breaks. Everything works fine locally. It seems that there is a 404 GET request for a https://domain.com/src/theme file which obviously doesn't exist.

The generated css still seems to include the variable names instead of the values from the theme file, which breaks the layout: .message { font-weight: bold; } .errorMessage { color: $color-error-font;}

@timneutkens
Copy link
Member

Can you provide a repository so I can check it out easily?

@timneutkens
Copy link
Member

Can you try next@canary

@jakegibson
Copy link
Author

Same issue on canary, I'll try to make a repo tomorrow

@codinronan
Copy link

Hey guys, any updates on this? Still happening in 6.0.3

@timneutkens
Copy link
Member

It’s close to impossible to help if you don’t provide an example repository. Saying something doesn’t work while the issue is still open and asking for updates without providing a way to reproduce doesn’t help solve the issue. Instead try providing a repository to reproduce, like I asked for already in this issue 👍🏻

@codinronan
Copy link

@timneutkens Given vercel/styled-jsx#425 and #4299 and #4335 I would have thought you were aware of the issue since you replied to them and at least one of those seems like the same issue, and the suggestion to use Canary also suggests you guys thought you had a fix. I didn't jump on to a thread, splatter my brains over it and leave.

I am building a repo now.

@jakegibson
Copy link
Author

Sorry work is crazy and I haven't gotten a chance to setup a repo, but a couple notes. Not using the global option <style jsx global> fixed it for everything except production builds. (Using static generated)

@blackbing
Copy link

I have same problem. and @timneutkens point out my problem. check out this.: #4239 (comment)

If you use babel-node check out this: #4239 (comment)

thanks to @timneutkens

@codinronan
Copy link

Echoing @blackbing and @timneutkens, that change to the babel config fixed everything on my side. @jakegibson unless you have further issues I am willing to bet this fixes it for you too and can close the issue.

Specifically this is the change that fixed it: #4239 (comment)

@timneutkens
Copy link
Member

Awesome, I'll just close this. If it doesn't work feel free to provide a full reproduction.

Thank you for verifying @codinronan @blackbing! ❤️

@blackbing
Copy link

blackbing commented May 31, 2018

@timneutkens sorry I think this issue should reopen. It could reproduce on this repo: https://github.com/blackbing/next-with-jest-style-jsx-error

and I also create an issue #4501 for "with-jest + with-style-jsx-scss" error.

the only difference of configs between test and development is

"preset-env": {
   "modules": "commonjs"
},

so it can reproduce if run NODE_ENV=test yarn dev, will show the same error with @jakegibson mentioned.

@jakegibson please help to confirm if it is the same error you got.

@codinronan
Copy link

codinronan commented May 31, 2018

@blackbing yep I can repro that as well, but I think the issue is not in next - I believe it is a problem with styled-jsx. Truthfully I'm not sure where to start there; saying that, styled-jsx 3 is almost a full rewrite from v2, so it may not repro in the new version. That would be nice!

@timneutkens
Copy link
Member

@giuseppeg I'm guessing this is because they transpile import/export with Babel before styled-jsx touches it right 🤔

@giuseppeg
Copy link
Contributor

@timneutkens probably, the commonjs plugin is not rewriting style correctly:

var _style2 = _interopRequireDefault(require("./style"));

function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }

var _default = function _default() {
  return _react.default.createElement("div", {
    className: "hello"
  }, _react.default.createElement("p", null, "Hello World"), _react.default.createElement(_style.default, {
    styleId: _style2.default.__hash,
    css: style
  }));
};

see the 3rd last line css: style, maybe @blackbing @codinronan or @jakegibson can submit a PR to styled-jsx with a failing test?

In the meantime you folks can switch to the commonjs syntax for styled-jsx:

module.exports = css`div { color: red }`

fwiw the issue is not with styled-jsx-plugin-sass

blackbing added a commit to blackbing/next-with-jest-style-jsx-error that referenced this issue May 31, 2018
@blackbing
Copy link

ok, I've update the repo and narrow down the problem. https://github.com/blackbing/next-with-jest-style-jsx-error

@blackbing
Copy link

@giuseppeg I can help to submit a PR, but I'm not sure where should I add it? Is it in next.js or styled-jsx?

@giuseppeg
Copy link
Contributor

@blackbing styled-jsx You can add the test here https://github.com/zeit/styled-jsx/blob/master/test/index.js
Install babel-preset-env as a dev dependency and use transform like this:

await transform('./fixtures/somefixture.js', { presets: [["babel-preset-env", { "modules": "commonjs" }]] })

blackbing added a commit to blackbing/styled-jsx that referenced this issue Jun 1, 2018
@blackbing
Copy link

@giuseppeg it looks pass. blackbing/styled-jsx@0feb15f or do I miss something?

@giuseppeg
Copy link
Contributor

yea it seems to be correct. Probably in Next.js preset-env is transforming things too early.
I'd try to use styled-jsx as a preset and run it after preset-env. I left a comment on your branch

@lock lock bot locked as resolved and limited conversation to collaborators Jun 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants