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

Glob pattern issues in 1.7.1 #2908

Closed
icopp opened this issue Sep 27, 2017 · 14 comments
Closed

Glob pattern issues in 1.7.1 #2908

icopp opened this issue Sep 27, 2017 · 14 comments
Labels
locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. type:bug Issues identifying ugly output, or a defect in the program

Comments

@icopp
Copy link

icopp commented Sep 27, 2017

Input:

node_modules/.bin/prettier --parser json --write package.json

Output:

Unable to expand glob patterns: package.json !**/node_modules/** !./node_modules/**
TypeError: Cannot read property 'name' of undefined

Expected behavior:

Prettier runs and parses the given file

@suchipi
Copy link
Member

suchipi commented Sep 27, 2017

I'm unable to reproduce; could you provide more info? What node version are you using?

@ikatyang
Copy link
Member

I cannot reproduce, this is how I tested:

mkdir test && cd test && yarn --init && yarn add prettier && node_modules/.bin/prettier --parser json --write package.json

Can you share steps to reproduce?

@icopp
Copy link
Author

icopp commented Sep 27, 2017

node -v
v8.5.0

npm -v
5.4.2

Interestingly, making a new repo and testing there works fine. However, on my existing repo, 1.7.0 works fine while 1.7.1 fails. I'm trying to figure out what the difference is, but so far I can only think that there must be a dependency getting pulled in with a slightly different version that must be mucking things up.

@ikatyang
Copy link
Member

Prettier bundled all the dependencies before publishing to npm, so that it's impossible to be affected by dependencies.

https://www.npmjs.com/package/prettier: 0 dependencies

@ikatyang
Copy link
Member

I'm able to reproduce something like this in #2924 with single line js config:

mkdir test
cd test && yarn --init
yarn add prettier
echo 'module.exports = { semi: true };' > myConfig.js
echo 'const x = 1' > index.js
node_modules/.bin/prettier --config myConfig.js index.js

cosmiconfig parses myConfig.js as yaml (parsed = { "module.exports = { semi": "true };" }) causing invalid config error catched by this try...catch:

  try {
    const filePaths = globby
      .sync(patterns, { dot: true })
      .map(
        filePath =>
          path.isAbsolute(filePath)
            ? path.relative(process.cwd(), filePath)
            : filePath
      );
    if (filePaths.length === 0) {
      console.error(`No matching files. Patterns tried: ${patterns.join(" ")}`);
      process.exitCode = 2;
      return;
    }
    ignorer
      .filter(filePaths)
      .forEach(filePath =>
        callback(filePath, getOptionsForFile(argv, filePath)) // <-----------------
      );
  } catch (error) {
    throw error;
    console.error(
      `Unable to expand glob patterns: ${patterns.join(" ")}\n${error}` // <-------------
    );
    // Don't exit the process if one pattern failed
    process.exitCode = 2;
  }

Not sure if this is your case, can you share your config setting?

@bd82
Copy link
Contributor

bd82 commented Sep 28, 2017

I have the same issue, tried creating a reproducing repo and surprisingly everything works...
I have a suspicion this is related to the file structure of he project.

@bd82
Copy link
Contributor

bd82 commented Sep 28, 2017

Reproducible example here:
https://github.com/bd82/prettier_172_issue

The issue seems related to the existence of a "prettier" section in the package.json file.

I would hazard a guess this has something to do with default options behavior.

@ikatyang
Copy link
Member

@bd82

It seems you're using incorrect config, no-semi is for CLI, you should write semi: false there, it'll just work fine, but we should show better warning message.

@bd82
Copy link
Contributor

bd82 commented Sep 28, 2017

Thanks. @ikatyang

Was this changed? as the no-semi option was definitely working in 1.6

@ikatyang
Copy link
Member

ikatyang commented Sep 28, 2017

@bd82

Maybe it's caused by our original implementation, but it's never mentioned on official docs before, so it shouldn't be an issue.

@ikatyang
Copy link
Member

ikatyang commented Sep 28, 2017

I believe @icopp's issue is something like I mentioned above, opened cosmiconfig/cosmiconfig#95 to track it.

@ikatyang ikatyang added type:bug Issues identifying ugly output, or a defect in the program scope:dependency Issues that cannot be solved inside Prettier itself, and must be fixed in a dependency and removed Needs More Information labels Sep 28, 2017
@icopp
Copy link
Author

icopp commented Sep 28, 2017

It's definitely the config file doing it.

When I have a .prettierrc file with these options, it breaks:

{
  "eslintIntegration": true,
  "semi": false,
  "singleQuote": true
}

When I have no such file, it works.

The eslintIntegration option is used by some editors as a setting lookup, and previously didn't cause any trouble.

@ikatyang
Copy link
Member

OK, I think #2929 should fix it, sorry for causing such issue, I forgot there may be some unknown options while implementing the normalization.

@ikatyang ikatyang removed the scope:dependency Issues that cannot be solved inside Prettier itself, and must be fixed in a dependency label Sep 28, 2017
@bd82
Copy link
Contributor

bd82 commented Sep 29, 2017

Thanks @ikatyang

@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Jul 6, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Jul 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. type:bug Issues identifying ugly output, or a defect in the program
Projects
None yet
Development

No branches or pull requests

4 participants