Skip to content

fixed css-loader instalation and css import in example files #1634

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jun 25, 2024
Merged

fixed css-loader instalation and css import in example files #1634

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
546bc0e
fixed css-loader instalation and css import in example files
vaukalak Jun 22, 2024
d1ae13c
Update CHANGELOG.md
justin808 Jun 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ Please follow the recommendations outlined at [keepachangelog.com](http://keepac
### [Unreleased]
Changes since the last non-beta release.

#### Fixed
- Fixed css-loader installation with [PR 1634](https://github.com/shakacode/react_on_rails/pull/1634) by [vaukalak](https://github.com/vaukalak)
Comment on lines +21 to +22
Copy link
Contributor

@coderabbitai coderabbitai bot Jun 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure proper spacing around headings and list items.

The markdown linting errors indicate missing blank lines around headings and list items. This can affect readability and the proper rendering of the document.

21a22
> 
22a24
>
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#### Fixed
- Fixed css-loader installation with [PR 1634](https://github.com/shakacode/react_on_rails/pull/1634) by [vaukalak](https://github.com/vaukalak)
#### Fixed
- Fixed css-loader installation with [PR 1634](https://github.com/shakacode/react_on_rails/pull/1634) by [vaukalak](https://github.com/vaukalak)
Tools
Markdownlint

21-21: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines

22-22: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines

22-22: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines

#### Fixed
Copy link
Contributor

@coderabbitai coderabbitai bot Jun 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid duplicate headings.

The use of multiple identical headings ('Fixed') within the same section is flagged. Consider merging these or rephrasing to maintain clarity and structure.

- #### Fixed

Committable suggestion was skipped due to low confidence.

Tools
Markdownlint

23-23: Expected: 1; Actual: 0; Above (MD022, blanks-around-headings)
Headings should be surrounded by blank lines

23-23: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines

23-23: null (MD024, no-duplicate-heading)
Multiple headings with the same content

- Address a number of typos and grammar mistakes [PR 1631](https://github.com/shakacode/react_on_rails/pull/1631) by [G-Rath](https://github.com/G-Rath)

Expand Down
5 changes: 3 additions & 2 deletions lib/generators/react_on_rails/base_generator.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ def copy_base_files
def copy_js_bundle_files
base_path = "base/base/"
base_files = %w[app/javascript/packs/server-bundle.js
app/javascript/bundles/HelloWorld/components/HelloWorldServer.js]
app/javascript/bundles/HelloWorld/components/HelloWorldServer.js
app/javascript/bundles/HelloWorld/components/HelloWorld.module.css]
base_files.each { |file| copy_file("#{base_path}#{file}", file) }
end

Expand Down Expand Up @@ -91,7 +92,7 @@ def add_yarn_dependencies

puts "Adding CSS handlers"

run "yarn add css-loader css-minimizer-webpack-plugin mini-css-extract-plugin style-loader@"
run "yarn add -D css-loader css-minimizer-webpack-plugin mini-css-extract-plugin style-loader"

puts "Adding dev dependencies"
run "yarn add -D @pmmmwh/react-refresh-webpack-plugin react-refresh"
Expand Down
2 changes: 1 addition & 1 deletion ..._on_rails/templates/base/base/app/javascript/bundles/HelloWorld/components/HelloWorld.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import PropTypes from 'prop-types';
import React, { useState } from 'react';
import style from './HelloWorld.module.css';
import * as style from './HelloWorld.module.css';
Copy link
Member

@justin808 justin808 Jun 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://www.perplexity.ai/search/Im-using-latest-upydKTUDSP.1jhdYmW5tGw#0

When using CSS Modules with webpack, the choice between import style from './some-css-file.module.css' and import * as style from './some-css-file.module.css' depends on how the CSS loader is configured and what you want to do with the imported styles. Here's a breakdown of the differences:

  1. import style from './some-css-file.module.css':

This syntax is typically used when the CSS loader is configured with modules: true and namedExport: false (which is the default)[4]. It imports the default export of the CSS module, which is an object containing all the class names as properties.

Benefits:

  • More concise syntax
  • Directly accesses the class names as object properties

Example usage:

import style from './some-css-file.module.css';
console.log(style.className); // Outputs the generated class name
  1. import * as style from './some-css-file.module.css':

This syntax is often used when the CSS loader is configured with modules: { namedExport: true }[4]. It imports all exported bindings from the CSS module as properties of the style object.

Benefits:

  • Allows for named exports of individual class names
  • Can be useful when you want to destructure specific class names

Example usage:

import * as style from './some-css-file.module.css';
console.log(style.className); // Outputs the generated class name

// Or with destructuring:
import { className1, className2 } from './some-css-file.module.css';

In most cases, the first syntax (import style from ...) is preferred because:

  1. It's more commonly used and understood by developers[5].
  2. It works with the default CSS Modules configuration in webpack.
  3. It provides a clear namespace for your styles, reducing the chance of naming conflicts.

However, the second syntax (import * as style from ...) can be useful in specific scenarios, such as when you need to dynamically access class names or when you're working with a build system that expects named exports.

Ultimately, the choice depends on your project's specific needs and your team's preferences. Both approaches are valid and will work with properly configured CSS Modules in webpack[1][4].

Citations:
[1] webpack/css-loader#253
[2] https://stackoverflow.com/questions/44474484/how-to-import-css-files-into-webpack
[3] https://webpack.js.org/loaders/css-loader/
[4] https://webpack.js.org/loaders/style-loader/
[5] https://blog.logrocket.com/how-to-configure-css-modules-webpack/

Copy link
Member

@justin808 justin808 Jun 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/webpack-contrib/css-loader/blob/master/CHANGELOG.md#-breaking-changes


const HelloWorld = (props) => {
const [name, setName] = useState(props.name);
Expand Down
2 changes: 1 addition & 1 deletion ...on_rails/templates/redux/base/app/javascript/bundles/HelloWorld/components/HelloWorld.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import PropTypes from 'prop-types';
import React from 'react';
import style from './HelloWorld.module.css';
import * as style from './HelloWorld.module.css';
Copy link
Contributor

@coderabbitai coderabbitai bot Jun 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tip

Codebase Verification

Incorrect import statement for CSS module.

The HelloWorld.module.css file does not contain named exports, which means the namespace import (import * as style) is unnecessary. It would be more appropriate to use a default import for the CSS module.

  • File: lib/generators/react_on_rails/templates/redux/base/app/javascript/bundles/HelloWorld/components/HelloWorld.jsx
  • Line: 3
  • Suggested Change: import style from './HelloWorld.module.css';
Analysis chain

Corrected import statement to use namespace import for CSS module.

This change aligns with common practices for importing CSS modules in React applications, especially when the CSS file contains multiple named exports. Ensure the HelloWorld.module.css is structured accordingly.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the CSS module supports namespace import.

# Test: Search for multiple named exports in the CSS module.
rg --type css 'export' lib/generators/react_on_rails/templates/redux/base/app/javascript/bundles/HelloWorld/components/HelloWorld.module.css

Length of output: 298

Script:

#!/bin/bash
# Description: Locate the CSS module file for HelloWorld component.

# Search for the CSS module file within the project directory.
fd --glob 'HelloWorld.module.css'

Length of output: 150

Script:

#!/bin/bash
# Description: Verify the structure of the CSS module file to support namespace import.

# Test: Search for multiple named exports in the CSS module.
rg --type css 'export' lib/generators/react_on_rails/templates/base/base/app/javascript/bundles/HelloWorld/components/HelloWorld.module.css

Length of output: 139

Script:

#!/bin/bash
# Description: Display the contents of the CSS module file for manual inspection.

# Display the contents of the CSS module file.
cat lib/generators/react_on_rails/templates/base/base/app/javascript/bundles/HelloWorld/components/HelloWorld.module.css

Length of output: 175

Copy link
Member

@justin808 justin808 Jun 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vaukalak can you explain the nature of this change?

Copy link
Contributor

@coderabbitai coderabbitai bot Jun 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Member

@justin808 justin808 Jun 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/webpack-contrib/css-loader/blob/master/CHANGELOG.md#-breaking-changes explains this.


const HelloWorld = ({ name, updateName }) => (
<div>
Expand Down