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

CSS Modules with TypeScript: css-modules-typescript-loader #5677

Closed
jednano opened this issue Nov 1, 2018 · 53 comments
Closed

CSS Modules with TypeScript: css-modules-typescript-loader #5677

jednano opened this issue Nov 1, 2018 · 53 comments
Assignees

Comments

@jednano
Copy link

jednano commented Nov 1, 2018

TypeScript users are flying blind when they import a CSS module. For example, if they type styles.foo they have no idea if .foo actually exists in the CSS module without manually checking the file.

This is not a great TypeScript experience.

I propose adding the css-modules-typescript-loader (or something similar) to emit type declarations for CSS modules on the fly.

It could be as simple as this:

  const loaders = [
    require.resolve('style-loader'),
    ...(useTypeScript && cssOptions.modules
      ? require.resolve('css-modules-typescript-loader')
      : []
    ),

What do you think?

@Timer
Copy link
Contributor

Timer commented Nov 1, 2018

I'd love to see a more formal proposal put together for this.

A couple things:

  1. This file shouldn't be seen by the user, it's unnecessary diff noise
  2. It needs to happen as a preloader on CSS Module files
  3. Gain a better understanding of caveats, I believe we opted against this initially.

/cc @brunolemos @ianschmitz

@jednano
Copy link
Author

jednano commented Nov 2, 2018

  1. It would be excellent if it could be transparently generated, but the file has to be there in order for the TypeScript compiler to understand the structure of the CSS module, so what do you think about adding *.module.d.ts files to the project root's .gitignore to clean up the diffs?
  2. Ideally, it would actually happen on file save of the *.module.css file, so maybe this is more of a "Run on Save" command? Otherwise, when you switch from the CSS module to the TypeScript file that imports it, it will still be blind.
  3. I'd love to know the reasons for which y'all opted against this functionality.

@Timer
Copy link
Contributor

Timer commented Nov 2, 2018

  1. It can be hidden away in node_modules or something, no?
  2. Run on save couldn't be safely configured, IMO. Thus the preloader.

@jednano
Copy link
Author

jednano commented Nov 2, 2018

  1. No. Pretty sure it has to be adjacent to the file in question and it has to have the same file name as the CSS module, so if the CSS module is named foo.module.css the definition file needs to be named foo.d.ts.
  2. I agree that would be a tough sell. Not sure what you're envisioning for the preloader, but I'm thinking that probably won't work.

@Timer
Copy link
Contributor

Timer commented Nov 2, 2018

  1. We can try both ways.
  2. Look at our ESLint loader config, will happen on save while dev server is running and consequently before type checking.

@jednano
Copy link
Author

jednano commented Nov 2, 2018

2️⃣ would be a better option, I agree. I don't think most people want to see this generated file.

@mrmckeb
Copy link
Contributor

mrmckeb commented Nov 2, 2018

Could this be implemented as a plugin to TypeScript? It would be great if this happened in-memory, as opposed to needing written files.
https://github.com/Microsoft/TypeScript/wiki/Writing-a-Language-Service-Plugin

@jednano
Copy link
Author

jednano commented Nov 2, 2018

@mrmckeb yes like css-module-types. Would a PR be accepted to CRA with this behavior?

@Timer
Copy link
Contributor

Timer commented Nov 2, 2018

Wow, that's rad! The lack of maintenance/use is a little concerning.

@jednano
Copy link
Author

jednano commented Nov 2, 2018

@Timer, I totally agree. Be nice to see some tests too. Internally, it looks like it's just a PostCSS plugin, which is a pretty good way of going about it.

@jamsch
Copy link

jamsch commented Nov 2, 2018

Right now I'm just using the vscode css-modules extension. It works pretty well but won't warn you for invalid styles.

@jednano
Copy link
Author

jednano commented Nov 3, 2018

The glory of this feature request is that it would work in any TypeScript-supported editor. @jamsch do you mean it doesn't warn you if you attempt to use a class name that isn't defined in the CSS module file?

@mrmckeb
Copy link
Contributor

mrmckeb commented Nov 4, 2018

@jedmao, if we don't get a response on css-module-types this week, I'll fork the project and add take ownership/responsibility.

  • Add tests
  • Add support for SCSS

@jednano
Copy link
Author

jednano commented Nov 4, 2018

@mrmckeb when you say SCSS support are you talking about nesting, specifically?

@mrmckeb
Copy link
Contributor

mrmckeb commented Nov 4, 2018

Correct, @jedmao. It also looks like the module specifically targets CSS (*.CSS).

@jednano
Copy link
Author

jednano commented Nov 4, 2018

Simple enough with postcss-nested.

@jednano
Copy link
Author

jednano commented Nov 4, 2018

Should we target *\.module\.[ps]?css? Really, the target should be configurable.

@mrmckeb
Copy link
Contributor

mrmckeb commented Nov 4, 2018

I think anything that CRA supports. For now it seems that it supports *.(sc|sa|c)ss.

@jednano
Copy link
Author

jednano commented Nov 4, 2018

Just to be clear, we would only ever want this functionality to activate when a CSS Module, specifically, is imported, so checking for module in the file name is relevant for CRA, as that's how it supports CSS Modules.

@mrmckeb
Copy link
Contributor

mrmckeb commented Nov 5, 2018

Of course ;) I just omitted that in my example.

I've set some time aside on Saturday to work on this if everyone is OK with what we've discussed?

As I understand it, the current plan is:

  • Support for all supported formats *\.module\.(sc|sa|c)ss
  • Tests for the plugin
  • Only add the plugin when CSS modules are in use (I'll need to investigate this)
  • Update docs

Let me know what else is needed.

@jednano
Copy link
Author

jednano commented Nov 5, 2018

@mrmckeb in response to bullet point 3️⃣, you're looking for useTypeScript && cssOptions.modules, as referenced in the OP.

Another thing I want to point out that css-modules-types repeatedly does this thing where it hijacks built-in ts functions with named args and then calls the original function with those specific args. I think it would be better to pass any/all incoming args ...args: any[] directly to the original ts function (i.e., ts.originalFunction(...args).

const originalFoo = ts.foo;
ts.foo = (...args) => {
    const [ first, second ] = args;
    // do stuff with `first` and `second`, the only args I'm using
    originalFoo(...args);
};

@mrmckeb
Copy link
Contributor

mrmckeb commented Nov 11, 2018

@jedmao Just an update, I made a start on this over the weekend but need a little more time. I'm hoping to find a streamlined approach. I'll also use plugin options to allow us to limit it's use in this project.

@dawnmist
Copy link

Question - Why does this need to be limited to *.modules.(sc|sa|c)ss files only? If a normal css file is imported with :local(.className) definitions, webpack class name rewriting still occurs (at least in Webpack 3 - we haven't tested 4 yet) and the typescript code still needs to be able to access styles.className.

We've been using typed-css-modules to autogenerate *.css.d.ts files from *.css files with react-scripts-ts 2.16 (and many prior versions) for more than a year now, and appears to be an attempt to solve the same issue (though with the downside that the generated files are added to git too). Is there some restriction in Webpack 4 that breaks the :local() scoping of a css class declaration? If not - why is module-only css filenaming being forced for something that should be applicable to either?

@Timer Timer added this to the 3.0 milestone Nov 13, 2018
@mrmckeb
Copy link
Contributor

mrmckeb commented Nov 13, 2018

Hi @dawnmist, we're aligning with the in-built CRA 2.0 behaviour - which requires *.module.*. Changing that behaviour would be a different discussion.

I have also tried typed-css-modules, but I think that this solution should be preferable to everyone as it doesn't actually require files to be generated and stored - instead, it happens in-memory. Please let us know if you see any disadvantages with this approach.

@jednano
Copy link
Author

jednano commented Nov 13, 2018

The only disadvantage I can think of is the scenario in which you actually want to generate/store the .d.ts files, because you're creating a component library of sorts. You don't want to supply any actual styles, but you do want to define what classes the styles should have in them? Sounds like an edge case though. You can also define your component styles in an interface within the component.

@dawnmist
Copy link

@mrmckeb CRA 2.0 still supports *.css files, and the documentation still refers to *.css files as being able to be imported. I'd believed that the new css-modules support was an added feature to CRA 2.0 rather than either a moved feature or representing a removed feature. Is this not the case? If the :local(.className) syntax is no longer supported in *.css files (as it had been previously in CRA 1.x with css-loader) it means that in order to update we'll have somewhere in the vicinity of 200+ *.css files that will need to be renamed and have the :local syntax removed from every definition within them in order to move forward. This requirement certainly wasn't listed in the breaking changes for CRA in any 2.0+ releases (which it should have been if there was a major removal of supported syntax). If the :local(.className) syntax isn't removed, then using it in a *.css file would still be in-built CRA behaviour (as it always was). It was only the generation of types that wasn't in-built previously (as CRA has only very recently added any official typescript support at all). Css-modules enable you to leave out the :local scoping for classes as they get loaded as local by default...but I didn't think it was intended that anyone already using CRA prior to 2.0 would be forced to migrate to using the module file definition if they were already using the scoped definitions available/supported previously before they could update to CRA 2.0+.

@mrmckeb
Copy link
Contributor

mrmckeb commented Nov 13, 2018

You're right, @dawnmist *.css is still supported. However, CSS modules (importing an object) need to have *.module.*. This is the new CRA 2.0 behaviour.

See here for more information on how this works:

// Adds support for CSS Modules (https://github.com/css-modules/css-modules)

I've also used *.css, and not *.module.css, but for me it wasn't a deal-breaker to rename files. You'll be able to run a bulk script in bash or your IDE to update file names and it would be a very quick change.

There are many great things that came with CRA 2.0, and the TS support in 2.1+ is an amazing addition. Don't forget you can also create your own fork of react-scripts (as we did) and customise the webpack config a little - in this case you could remove the requirement for *.module.* for your project :)

If you think this is something you'd like to see changed in CRA, you could open an issue for that discussion and your proposal to improve it. I think you'd get some support behind your thinking.

@mrmckeb
Copy link
Contributor

mrmckeb commented Nov 21, 2018

@jedmao @Timer I'm struggling to work out the best path forward here.

When create-react-app is installed, and react-scripts's init script is executed, there is no way to know that a user wants to use CSS modules. We can infer that they want to use TypeScript if they use --typescript.

Options
We can detect if a user is using CSS modules during react-scripts's start script - in the same way that we check if the user has *.(ts|tsx) a files present - and that's where I've gotten to. At this stage, we can do one of the following:

  1. Simply notify the user about the plugin, and how they can install it (as it works for TypeScript now if not installed)
  2. Notify the user about the plugin, install the plugin, and then add to tsconfig.json.
  3. Ask the user what they would like to do, then proceed as in option 2.

Option 3 seems ideal. However, there isn't a permanent opt-out here as we don't have a way to store user preferences:

  • We could add a react-scripts field to package.json to manage user settings for CRA, but that's a big change. However, this may be useful in other cases too.
  • We could just run it every time (until the user installs), but that's very annoying.

In ideas would be very welcome.

@jednano
Copy link
Author

jednano commented Nov 21, 2018

This is just a dev dependency, so I would install it regardless of whether the user will use it or not. This is how CSS Module support works for CRA too. After that, all you need is to conditionally load the plugin as I demonstrated in my original comment.

@mrmckeb
Copy link
Contributor

mrmckeb commented Nov 21, 2018

@jedmao Here's a rough example:
master...mrmckeb:feature/typescript-css-modules

I can finish it very quickly from here, but want to get your thoughts on the direction. The most notable issue is that we need to reference the module as ['.node_modules/react-scripts/node_modules/typescript-plugin-css-modules'] - because we can't be sure that it's available at root.

The alternative is to add it here, so we can just add to tsconfig.json as ['typescript-plugin-css-modules'].

@jednano
Copy link
Author

jednano commented Nov 21, 2018

@mrmckeb I'm not sure it has to be available at root, so long as it's listed in react-scripts/package.json and then in webpack.config.js. See how the style-loader is imported here.

I'm afraid I can't help more than that. I'm not that intimately familiar w/ this project.

@mrmckeb
Copy link
Contributor

mrmckeb commented Nov 22, 2018

A PR for this is now complete. It might needs some docs. Let me know what you think!

@timothykang
Copy link

@jedmao @mrmckeb Sorry for not responding to your issues (#1 #2) sooner over at css-modules-types...it looks like GitHub doesn't notify about new issues (or at least I don't know how to turn that on).

The main reason I stopped development on the plugin was that the dynamic type definitions are only available when editing, and not when compiling. I see that @mrmckeb has since created a fork and discovered this for himself. I would gladly pick up development on this idea again if/when that changes. Until then, good luck and feel free to ping me if there's anything I can do to help.

@mrmckeb
Copy link
Contributor

mrmckeb commented Nov 25, 2018

Thanks @timothykang, I've added a few features on top of what you had done - great work by the way. It's a blessing and a curse that plugins are IDE only for now... I think that'll change in future.

@jleider
Copy link

jleider commented Dec 12, 2018

@mrmckeb @timothykang I've been searching all day for something similar to this. The best things I can find at the moment are these three projects and the latter two depend on the former:

The first two options seem to be the ones with the fewest drawbacks while the loader one has the unfortunate design flaw of a race condition between creation of the *.scss.d.ts files and tsc requiring them for compilation.

While none of these options are a panacea by themselves, combining one of them with some other processes I think we can get what we are all after, stronger types at compile time from CSS to TS.

For example, suppose if Grunt/Gulp were to watch *.(css|scss|sass|less) files for any changes, I believe one could simply run typed-css-modules as an additional step directly after each SCSS/SASS/LESS compile to output the generated *.scss.d.ts files. This would execute similarly to how a linter runs, it would just be ordered after a compile. Then Webpack/TSC/etc could be configured to watch for those additional *scss.d.ts files and type check any *.ts files accordingly without any special plugins or transformers.

I believe having those extra *.scss.d.ts files written to the file system would satisfy both the IDE and compilation type enforcement requirements.

However, one downside to every one of these CSS to JS module libraries I have come across is that none of them seem to handle traditional CSS nesting. The only way I've seen one handle nesting is with BEM naming convention which you would rarely if ever use with SCSS/SASS/LESS.

@mrmckeb
Copy link
Contributor

mrmckeb commented Dec 13, 2018

@jleider, not to self promote, but did you try https://github.com/mrmckeb/typescript-plugin-css-modules? The downside is that it's IDE only, but the good thing is that it doesn't require additional files in your directories, supports nesting, etc. My team are using it with CRA 2.1+.

We (at CRA) could also move to a different TypeScript compiler in future (not using babel-typescript) which would allow us to use plugins as transforms (an unofficial feature) - but as it's not official, I'm not sure if that's ideal. It's unfortunate that as of right now, this is not supported by TypeScript officially... I'd encourage you to post over there to show interest in this feature being added to TypeScript.

@jleider
Copy link

jleider commented Dec 13, 2018

@mrmckeb I did take a look at your plugin, unfortunately an IDE only solution wont work for my team since not everyone uses an IDE during development so we really need a compile time solution. I think I am going to try the solution I proposed in my last #5677 (comment).

@mrmckeb
Copy link
Contributor

mrmckeb commented Dec 13, 2018

Of course @jleider, as I said it's unfortunate that there isn't another solution right now. This may also be added to the Babel plugin in future... we'll keep thinking of a better way to handle this.

@pristas-peter
Copy link

pristas-peter commented Dec 17, 2018

Hi guys, I wrote a webpack plugin for my personal use, which also watches files, which are not watched by webpack and runs its own compilation with the same webpack config, which is great for developer experience.

You can check it out here:
https://github.com/pristas-peter/react-webpack-utils/tree/master/packages/watcher-webpack-plugin
https://github.com/pristas-peter/react-webpack-utils/tree/master/packages/css-modules-typings-loader

Packages are also available at npm:
https://www.npmjs.com/package/watcher-webpack-plugin
https://www.npmjs.com/package/css-modules-typings-loader

If you find these useful, feel free to add PR if you need more functionality.

@Hotell
Copy link

Hotell commented Jan 8, 2019

This looks solid/mature enough (also baked by "huge" company): https://github.com/dropbox/typed-css-modules-webpack-plugin

@mrmckeb
Copy link
Contributor

mrmckeb commented Jan 8, 2019

@Hotell and @pristas-peter, thanks for sharing these.

As mentioned, at this stage we won't be adding a solution that writes additional files to disk. There are IDE-only solutions, like typescript-plugin-css-modules that work great with Create React App, but are editor-only for now.

Microsoft would need to allow TypeScript plugins/transforms to work outside of the IDE for us to be able to do this cleanly.

We definitely have this on our radar and will continue to monitor and assess as we plan the future of CRA.

@Hotell
Copy link

Hotell commented Jan 8, 2019

I put down quick gist for anyone reading this issue, if they desperately need this feature without ejecting :)

https://gist.github.com/Hotell/01035a3ec202245d6b97937444140877

I can send PR to update docs if you're willing to accept this :)

thanks!

@zxti
Copy link

zxti commented Feb 9, 2019

As a PSA for folks like me who hadn't realized this but are potentially interested - Webstorm has had intelligent support for Typescript importing CSS Modules (and sass modules) built-in since Jun 2017:

https://blog.jetbrains.com/webstorm/2017/06/webstorm-2017-2-eap-172-2953/

One of many reasons I switched over from VSCode a while ago!

@jednano
Copy link
Author

jednano commented Feb 9, 2019

@zxti, I'd rather this type of thing be an extension than built in. Best to keep vscode lean. That's one thing I love about it.

@zxti
Copy link

zxti commented Feb 9, 2019

@jedmao I actually highly agree with preferring modular extensibility. FWIW (and not to turn this thread too off-topic - only mentioned it initially because I thought others might be interested) this Typescript support is an extension to the Jetbrains IDE core (that's how I installed it - I don't actually use Webstorm-the-product, but just IntelliJ with these plugins installed). It's also nice to have features like renaming CSS classes, find-usages, etc. all work seamlessly (which I think these alternative solutions still have a ways to go on).

@babakness
Copy link

I've made a package that compiles SASS, w/ interpolations, includes, etc, and provides type definitions.

sass-module-types animation

https://github.com/babakness/sass-module-types

If one does not like the d.ts definition files, they can be hidden in VS Code, WebStorm, etc. In VS Code at the moment (Feb 2019) your definitions don't refresh if the file isn't open in the editor, so if you do hide the file you made need to "peek definition" to get the update to kick in. Worthy issue to bring up to the VS Code team.

@mrmckeb
Copy link
Contributor

mrmckeb commented Feb 25, 2019

Hi @babakness, thanks again for this.

Again, it's not something we'll add to CRA, but definitely worth checking out for those looking for a solution.

@mrmckeb mrmckeb self-assigned this Feb 25, 2019
@mrmckeb
Copy link
Contributor

mrmckeb commented Mar 20, 2019

Closing this off for now, but we definitely want to see a solution for this - and we'd love to hear any ideas that can help to solve this without writing additional files to disk.

Thanks for everyone's time looking into all the possible options here.

@mrmckeb mrmckeb closed this as completed Mar 20, 2019
@lock lock bot locked and limited conversation to collaborators Mar 25, 2019
@ianschmitz ianschmitz removed this from the 3.0 milestone Apr 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests