Update rcs core #44

X-Ryl669 · 2019-08-19T21:05:20Z

This update the code to work with version 3 of rcs-core.
It adds ignore feature to config file, so it's possible to ignore minified files (like those coming from external libraries)
It also adds source line tracking so generated warning are meaningful (to be used with rcs-core#94)

Don't allow non JSX file to run with JSX arguments

X-Ryl669 · 2019-08-19T21:15:38Z

I need help here, I don't know how to solve travis's report here.
I don't think it's clean to have a dependency on rcs-core^3.0.0-alpha.0 and I've tried rcs-core^3.0.0@next but it does not work either.

How to do this ?

JPeer264 · 2019-08-20T15:50:39Z

I don't think it's clean to have a dependency on rcs-core^3.0.0-alpha.0

True, that's why I am not releasing a stable version of rename-css-selectors based on an alpha rcs-core dependency. You have 3 options here to lock down to the current alpha version:

{
	"dependencies": {
		// locking the version
		"rcs-core": "^3.0.0-alpha.0",
		// locking the tag
		"rcs-core": "next",
		// locking the branch of github
		"rcs-core": "github:JPeer264/node-rcs-core#master",
		// locking any commit
		"rcs-core": "github:JPeer264/node-rcs-core#2815d48e6caa619a7cb38a5d014af7b762e6de6f"
    }
}

But I would suggest to lock down the version in this case. I will release an alpha version of rename-css-selectors as long as rcs-core doesn't have a stable version :)

JPeer264 · 2019-08-20T15:54:35Z

lib/process/replaceData.js

  if (
    options.type === 'js' ||
    (
      options.type === 'auto' &&
      fileExt.js.includes(path.extname(filePath))
    )
  ) {
-    data = rcs.replace.js(fileData, options.parserOptions);
+    // eslint-disable-next-line no-param-reassign


I think it still works without this line?

Will try. I don't know since I can't run test on my side.

coveralls · 2019-08-20T23:48:19Z

Coverage increased (+0.6%) to 97.026% when pulling 8d519fe on X-Ryl669:updateRCSCore into a10956d on JPeer264:master.

JPeer264

Some changes, plus I think my linting is not included into travis, I will update that. There might be some problems of the linter as well.

JPeer264 · 2019-08-21T18:15:02Z

lib/config/config.js

+    }
+    return false;
+  }
+}; // /include


could you remove ; // /include?

JPeer264 · 2019-08-21T18:21:03Z

lib/config/config.js

+  }
+}; // /include
+
+module.exports = new Config;


I actually don't know how that works, also I've never seen it. Could you add the parentheses to the class new Config();

You know if there are any differences between new Config and new Config(), just curious.

That's valid JS if there is not parameter to the constructor. The linter usually refuse it because it can lead to code like this: new Date.toString() which tries to invoke toString as the constructor of some object Date. It's usually not the developer intention. I'm changing it to be consistent.

Ah well, thanks for the answer. That makes sense :)

JPeer264 · 2019-08-21T18:24:37Z

package.json

-    "universalify": "^0.1.2"
+    "rcs-core": "^3.0.0-alpha.0",
+    "universalify": "^0.1.2",
+    "yarn": "^1.17.3"


Please do not add yarn as dependency :)

Either install yarn with homebrew (if you are on a mac) or install it globally via npm. I might switch fully to plain npm anyways soon.

Sorry, I used to use npm and when I've tried install yarn, it shouted that we shouldn't mix package manager, so I've removed the lock file and forgot to check the package.json file

No worries, all good.

X-Ryl669 · 2019-08-21T19:29:11Z

Should be OK now. I've fixed all original code that choked with the linter too.

JPeer264

I just saw two things - last changes then it's ready to merge :)

JPeer264 · 2019-08-21T20:03:52Z

lib/config/config.js

@@ -32,13 +32,13 @@ class Config {
  }

  isIgnored(filePath) {
-    for (let i = 0; i < this.ignorePatterns.length; i++) {
+    for (let i = 0; i < this.ignorePatterns.length; i += 1) {


I just saw this now. Could you change this method to following:

isIgnored(filePath) { return this.ignorePatterns.some(pattern => minimatch(filePath, pattern); }

JPeer264 · 2019-08-21T20:10:01Z

lib/mapping/loadMapping.js

@@ -2,38 +2,47 @@ const merge = require('lodash.merge');
 const rcs = require('rcs-core');
 const json = require('json-extra');

-const loadMapping = (pathString, options) => {
+function loadReversed(selectors) {


Also here please change to following:

const loadReversed = selectors => ( Object.entries(selectors).reduce((prev, [key, value]) => ({ ...prev, [key.charAt(0) + value]: key.slice(1, key.length), }), {}) );

Nope. ava or nyc does not accept object destructuring IIRC (I had to use _.merge instead).

Not sure it's more understandable this way anyway.
I'm going to do something in between without reduce

Edit: Yep, it does not work:

SyntaxError: /Users/cyril/esp/node-rename-css-selectors/lib/mapping/loadMapping.js: Unexpected token (7:4) 5 | const loadReversed = selectors => ( 6 | Object.entries(selectors).reduce((prev, [key, value]) => ({ > 7 | ...prev, | ^ 8 | [key.charAt(0) + value]: key.slice(1, key.length), 9 | }), {})); 10 | at Parser.pp$5.raise (/Users/cyril/esp/node-rename-css-selectors/node_modules/babylon/lib/index.js:4454:13) at Parser.pp.unexpected (/Users/cyril/esp/node-rename-css-selectors/node_modules/babylon/lib/index.js:1761:8) at Parser.pp$3.parseIdentifier (/Users/cyril/esp/node-rename-css-selectors/node_modules/babylon/lib/index.js:4332:10) at Parser.pp$3.parsePropertyName (/Users/cyril/esp/node-rename-css-selectors/node_modules/babylon/lib/index.js:4156:96) at Parser.pp$3.parseObj (/Users/cyril/esp/node-rename-css-selectors/node_modules/babylon/lib/index.js:4045:12)

X-Ryl669 · 2019-08-21T20:44:18Z

Let me know if it's ok for you this way. It's seem easier to read IMHO.
Also, please increment the version in rcs-core so I can add the new allWarnings.warn() call to get the final (merged) warning report on the console. I'm preparing a new PR for this on this project with some documentation update.

JPeer264 · 2019-08-22T06:03:29Z

I released rcs-core@3.0.0-alpha.1 or rcs-core@next 👍

* Update code to support changes in RCS core * Add support for true options from config file. Don't allow non JSX file to run with JSX arguments * Chore: install rcs-core@next * Fix all tests to run with the changes * Improve code coverage * Make linter happy! * Fix requested changes Co-authored-by: Jan Peer Stöcklmair <jan.oster94@gmail.com>

klimashkin · 2019-11-26T02:09:54Z

Hi! Not sure why it's in a patch update of the rename-css-selectors
It breaks my build after 3.2.5 -> 3.2.6 update. Something is replaced too greedy in js files

JPeer264 · 2019-11-26T06:13:54Z

Uff thats really bad. So "just" JS files are breaking?

It's in a patch as it shouldn't break the usage of rcs. I'm gonna invest in this asap!

JPeer264 · 2019-11-26T06:18:53Z

Btw which kinda errors are you getting? JS files are being replaced wrongly or you JS files where you use rcs are breaking? E.g. some methods are not found?

JPeer264 · 2019-11-26T06:34:50Z

FYI. I unpublished 3.2.6 and released 3.2.7 which is the same as 3.2.5, so nobody should experience any issues for now, until we solved the problem with the new rcs-core.

klimashkin · 2019-11-26T06:47:48Z

Thanks!
Some pieces of js files where replaced outside of strings, so it became not parsable anymore

X-Ryl669 · 2019-11-26T07:06:33Z

Can you give the example code that breaks (before and after transform) ? Thanks.

X-Ryl669 · 2019-11-28T22:18:46Z

@klimashkin It'll be hard to fix if you don't give us more details. Can you give (part of) the source JS code and the compiled JS code that has broken? Thanks.

JPeer264 · 2020-02-20T13:51:03Z

@klimashkin could you try v4.0.0-rc.0 or as alias rename-css-selectors@next and report if your error still exists? :)

X-Ryl669 added 3 commits August 16, 2019 21:03

Update code to support changes in RCS core

a9dcdef

Add support for true options from config file.

549022c

Don't allow non JSX file to run with JSX arguments

Merge branch 'master' into updateRCSCore

c410940

JPeer264 requested changes Aug 20, 2019

View reviewed changes

Chore: install rcs-core@next

a253504

JPeer264 mentioned this pull request Aug 20, 2019

Better error reporting JPeer264/node-rcs-core#94

Merged

X-Ryl669 force-pushed the updateRCSCore branch from c809454 to e17bf3a Compare August 20, 2019 23:46

Fix all tests to run with the changes

e17bf3a

Improve code coverage

7d94db1

JPeer264 requested changes Aug 21, 2019

View reviewed changes

JPeer264 and others added 3 commits August 21, 2019 20:40

Merge branch 'master' into updateRCSCore

08e3299

Merge branch 'master' into updateRCSCore

e610952

Make linter happy!

36faecf

JPeer264 requested changes Aug 21, 2019

View reviewed changes

Fix requested changes

8d519fe

JPeer264 merged commit c230f13 into JPeer264:master Aug 22, 2019

JPeer264 mentioned this pull request Nov 26, 2019

Messes up with the ~ character #48

Closed

Uh oh!

Update rcs core #44

Update rcs core #44

Uh oh!

Conversation

X-Ryl669 commented Aug 19, 2019

Uh oh!

X-Ryl669 commented Aug 19, 2019

Uh oh!

JPeer264 commented Aug 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coveralls commented Aug 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JPeer264 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

X-Ryl669 Aug 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

X-Ryl669 commented Aug 21, 2019

Uh oh!

JPeer264 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

X-Ryl669 Aug 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

X-Ryl669 commented Aug 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JPeer264 commented Aug 22, 2019

Uh oh!

klimashkin commented Nov 26, 2019

Uh oh!

JPeer264 commented Nov 26, 2019

Uh oh!

JPeer264 commented Nov 26, 2019

Uh oh!

JPeer264 commented Nov 26, 2019

Uh oh!

klimashkin commented Nov 26, 2019

Uh oh!

X-Ryl669 commented Nov 26, 2019

Uh oh!

X-Ryl669 commented Nov 28, 2019

Uh oh!

JPeer264 commented Feb 20, 2020

Uh oh!

Uh oh!

JPeer264 commented Aug 20, 2019 •

edited

Loading

coveralls commented Aug 20, 2019 •

edited

Loading

X-Ryl669 Aug 21, 2019 •

edited

Loading

X-Ryl669 Aug 21, 2019 •

edited

Loading

X-Ryl669 commented Aug 21, 2019 •

edited

Loading