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

fix: provide an unquoted path argument for opt.resolve(path) #127

Merged

Conversation

kamilic
Copy link
Contributor

@kamilic kamilic commented Jul 22, 2021

Argument path is a quoted string In opt.resolve(path) which is not convient for user to use this api.
Also, it is not an expected behaviour for pr #126.

For example:

.a {
  compose: b from "@/scss/compose.css";
}

Current behaviour:

postcssModules({
  resolve(path) {
     console.log(path) // "\"@/scss/compose.css\""
  }
})

Expected behaviour:

postcssModules({
  resolve(path) {
     console.log(path) // "@/scss/compose.css"
  }
})

Sorry for my mistake.

@kamilic kamilic force-pushed the provide-an-unquoted-path-argument branch 2 times, most recently from 36bc6ff to 5de1d03 Compare July 22, 2021 06:41
src/index.js Outdated Show resolved Hide resolved
@kamilic kamilic force-pushed the provide-an-unquoted-path-argument branch from 5de1d03 to ebc8218 Compare July 23, 2021 10:29
@kamilic
Copy link
Contributor Author

kamilic commented Jul 23, 2021

@madyankin done.

@@ -0,0 +1,16 @@
// copied from https://github.com/lakenen/node-unquote

var reg = /['"]/;
Copy link
Owner

Choose a reason for hiding this comment

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

I suspect this regexp will replace only the quotes and leave the backslashes. Shouldn't it be [\'\"]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

eslint tells me it can be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

Copy link
Owner

Choose a reason for hiding this comment

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

My bad, I have read the code not thorough enough the first time. Everything is ok

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@madyankin If you want to keep it like where copied from, I will change it to [\'\"] and add a // eslint-disable no-useless-escape line to disable this rule.

Copy link
Contributor Author

@kamilic kamilic Jul 23, 2021

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants