facebook / create-react-app

Set up a modern web app by running one command.
https://create-react-app.dev
MIT License
102.56k stars 26.8k forks source link

Support CSS Modules that are imported by JS modules in node_modules #5687

Closed penx closed 5 years ago

penx commented 5 years ago

EDIT: This has started working on codesandbox without any edits, will try figure out why.

Is this a bug report?

Yes

Did you try recovering your dependencies?

N/A

Which terms did you search for in User Guide?

N/A

Environment

codesandbox

Steps to Reproduce

Open: https://codesandbox.io/s/w0x8xk5lk

Or use the following code:

index.js

import React from "react";
import ReactDOM from "react-dom";
import Button from "./button";

function App() {
  return (
    <div>
      <Button>Test</Button>
    </div>
  );
}

const rootElement = document.getElementById("root");
ReactDOM.render(<App />, rootElement);

button.js

import React from "react";
import styles from "./button.module.scss";
// import styles from "govuk-frontend/components/button/_button.scss";
// import styles from "@penx/govuk-frontend/components/button/_button.module.scss";

const Button = props => (
  <button className={styles["govuk-button"]} {...props} />
);

export default Button;

button.scss

@import "~govuk-frontend/components/button/button";

.govuk-button {
  composes: govuk-button;
}

Notice that

import styles from "./button.module.scss";

and

import styles from "@penx/govuk-frontend/components/button/_button.module.scss";

Will correctly import the CSS module.

2.

Open https://codesandbox.io/s/6yk2prz14w

Or use the following code:

import React from "react";
import ReactDOM from "react-dom";
import { Button } from "govuk-frontend-react";

function App() {
  return (
    <div>
      <Button>Test</Button>
    </div>
  );
}

const rootElement = document.getElementById("root");
ReactDOM.render(<App />, rootElement);

Notice that

import { Button } from "govuk-frontend-react";

Throws the error:

Error: Could not find /node_modules/govuk-frontend/tools/exports
        on line 2 of node_modules/govuk-frontend/tools/_all.scss
>> @import "exports";
   --------^

Despite _exports.scss existing: https://github.com/alphagov/govuk-frontend/blob/master/src/tools/_exports.scss

These 2 sandboxes are doing more or less the same thing, but the second is doing it via a component library on npm https://github.com/penx/govuk-frontend-react

Expected Behavior

I would expect the 2 linked sandboxes above to exhibit the same behaviour, and for _exports.scss to be resolved.

Actual Behavior

The second sandbox fails, because it is importing a React component from npm, that in turn is importing a CSS module. I suspect that the webpack configuration means that @import "exports"; inside /node_modules/**/myfile.scss does not correctly resolve to /node_modules/**/_exports.scss

Reproducible Demo

As above

penx commented 5 years ago

Related

https://github.com/alphagov/govuk-frontend/issues/1052 https://github.com/UKHomeOffice/govuk-react/issues/76

Timer commented 5 years ago

I'm not sure how I feel about this. Packages that are published to npm shouldn't require you configure tooling to consume them.

Why can't these packages compile away their CSS module imports and replace them with actual CSS files & class names?

penx commented 5 years ago

I'm not sure how I feel about this. Packages that are published to npm shouldn't require you configure tooling to consume them.

I agree to a point, which is why we are using CSSinJS at the moment in govuk-react. However, it has become common for Sass CSS modules to be distributed on npm and I don’t see a downside to supporting this. If CRA supports loading Sass CSS Modules from an npm module, why not support a React wrapper for the CSS module too?

Why can't these packages compile away their CSS module imports and replace them with actual CSS files & class names

To retain the treeshaking, path splitting and critical CSS benefits that you get with CSS Modules or CSSinJS.

FWIW we're currently (partially) recreating govuk-frontend in govuk-react using CSSinJS (emotion) but want to introduce something that keeps the two projects in line. Converting Sass CSS modules to CSSinJs may be another option but not something we have cracked yet.

penx commented 5 years ago

For some reason the second codesandbox has started working without me touching it 😕

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

penx commented 5 years ago

I am still looking in to this and planning to build govuk-frontend-react on the basis that it would be supported by create-react-app. It appears to be working now so I can only assume it was resolved by a patch update.

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

stale[bot] commented 5 years ago

This issue has been automatically closed because it has not had any recent activity. If you have a question or comment, please open a new issue.