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

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

Closed
penx opened this issue Nov 2, 2018 · 8 comments
Closed

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

penx opened this issue Nov 2, 2018 · 8 comments
Labels

Comments

@penx
Copy link

penx commented Nov 2, 2018

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
Copy link
Author

penx commented Nov 2, 2018

@penx penx changed the title Support CSS Modules that are imported by code in node_modules Support CSS Modules that are imported by modules in node_modules Nov 2, 2018
@penx penx changed the title Support CSS Modules that are imported by modules in node_modules Support CSS Modules that are imported by JS modules in node_modules Nov 2, 2018
@Timer
Copy link
Contributor

Timer commented Nov 2, 2018

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
Copy link
Author

penx commented Nov 2, 2018

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
Copy link
Author

penx commented Nov 5, 2018

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

@stale
Copy link

stale bot commented Dec 5, 2018

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 stale bot added the stale label Dec 5, 2018
@penx
Copy link
Author

penx commented Dec 5, 2018

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 stale bot removed the stale label Dec 5, 2018
@stale
Copy link

stale bot commented Jan 6, 2019

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 stale bot added the stale label Jan 6, 2019
@stale
Copy link

stale bot commented Jan 12, 2019

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.

@stale stale bot closed this as completed Jan 12, 2019
@lock lock bot locked and limited conversation to collaborators Jan 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants