search

JPeer264 / node-rename-css-selectors

📝 Rename css classes and id's in files
MIT License
65 stars 9 forks source link

Ids and classes are treated as the same identity #69

Closed ghost closed 3 years ago

ghost commented 3 years ago

First thanks for the great module.

I noticed that RCS appears to be overriding identities if they are named the same, so this issue is about RCS creating a collision between normatively different css entities like ids and classes.

Example:

style-file-1.css:

.container {
width: 100%;
}

style-file-2.css

#container {
width: 800px;
}

index.html:

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="UTF-8" />
    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
    <title>Test</title>
  </head>
  <body>
    <div class="container">
      <div>hello</div>
      <div>world</div>
    </div>
    <div class="container">
      <div id="container">!</div>
    </div>
  </body>
</html>

code:

const rcsCore = require("rcs-core"),
  rcs = require("rename-css-selectors");

rcs.process
  .auto(["**/*.css", "**/*.html"])
  .then(() => {
    rcs.generateMapping("./", {
      overwrite: true,
    });
  })
  .catch((error) => {
    log(error);
  });

Results:

renaming-map.json:

{
".container": "ne",
}

index.html

<!DOCTYPE html><html lang="en"><head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>Test</title>
  </head>
  <body>
    <div class="ne">
      <div>hello</div>
      <div>world</div>
    </div>
    <div class="ne">
      <div id="ne">!</div> <!-- this should have a different replacement -->
    </div>

</body></html>

style-file-1.css:

.ne {
width: 100%;
}

style-file-2.css

.ne { /* rcs changed this from id to class */
width: 800px;
}

Expected behavior:

renaming-map.json:

{
".container":"ne",
"#container":"ab"
}

index.html

<!DOCTYPE html><html lang="en"><head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>Test</title>
  </head>
  <body>
    <div class="ne">
      <div>hello</div>
      <div>world</div>
    </div>
    <div class="ne">
      <div id="ab">!</div> 
    </div>

</body></html>

style-file-1.css:

.ne {
width: 100%;
}

style-file-2.css

#ab {
width: 800px;
}
ghost commented 3 years ago

I'm using

❯ yarn list rcs-core yarn list v1.22.5 warning Filtering by arguments is deprecated. Please use the pattern option instead. ├─ rcs-core@3.5.0 └─ rename-css-selectors@3.3.0 ----└─ rcs-core@2.6.3 ✨ Done in 0.15s. ❯ yarn list rename-css-selectors yarn list v1.22.5 warning Filtering by arguments is deprecated. Please use the pattern option instead. └─ rename-css-selectors@3.3.0 ✨ Done in 0.14s.

JPeer264 commented 3 years ago

Thanks for your issue. This seems to be a bug. To verify if that one is still occurring in the latest version, could you try the latest release v4.0.0-rc.2 or npm i rename-css-selectors@next

ghost commented 3 years ago

With 4.0.0-rc.2 it does correctly use a different renaming for the id and classes in the html and css files but it does not save the mapping for the id in renaming-map.json.

JPeer264 commented 3 years ago

Alright good to know. It seems I forgot to rewrite the mapping part here. Could you be so kind and write the config for now by yourself?

import rcsCore from 'rcs-core';
import fs from 'fs';

// better to use async methods but for the use case this is simpler
const loadedMapping = fs.readFileSync(YOUR_FILENAME, 'utf8')

// LOADING
rcsCore.mapping.load(loadedMapping);

// now do your rename-css-selectors magic
// e.g. rcs.process.auto(FILES)

// SAVING
const mappingToSave = rcsCore.mapping.generate();

// better to use async methods but for the use case this is simpler
fs.writeFileSync(YOUR_FILENAME, mappingToSave, 'utf8');
ghost commented 3 years ago

Yes no problem. Anyways having same names for both classes and ids is very rare. I can edit manually the mapping for now and load it in future renamings. Thanks a lot :)

JPeer264 commented 3 years ago

Please be aware that the mappings between rename-css-selectors v3 and v4 will be different (in v4.0.0-rc.3 and above

JPeer264 commented 3 years ago

But the approach I sent will work in the future with stable v4

ghost commented 3 years ago

I was wrong it is also saving the mapping for the id with your code above. I'm using

  const loadedMapping = JSON.parse(
    fs.readFileSync(path.resolve(YOUR_PATH, YOUR_FILENAME)) 
  );
  rcsCore.mapping.load(loadedMapping);

rcs.process
    .auto(["**/**/*.css", "**/**/*.html"], options)
    .then(() => {
        const mappingToSave = rcsCore.mapping.generate();
        fs.writeFileSync(YOUR_FILENAME, JSON.stringify(mappingToSave), "utf8");
        log("done")
    })
    .catch((error) => {
      log(error);
    });

Had to use .parse and .stringify to avoid errors: [Object object] is not an object in mapping.load(), and data argument must be string in writeFileSync. But it saves everything in the mapping. Thanks again.

JPeer264 commented 3 years ago

Oh ya sorry that was my mistake, I didn't thought about this one. I am always using the fs-extras writeJson and readJson function, I guess this was the reason I forgot that part :)

JPeer264 commented 3 years ago

@personaaleatoria FYI

You can now use the new mapping function directly from rename-css-selectors (this example with top-level await) with v4.0.0-rc.3:

import rcs from 'rename-css-selectors';
import fs from 'fs';

// LOADING
await rcs.mapping.load(YOUR_FILENAME);

// process some files
await rcs.process.auto(FILES)

// SAVING
await rcs.mappings.generate(YOUR_FILENAME, { overwrite: true });