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

Support all prettier config files #4874

Closed

Conversation

pablopunk
Copy link
Contributor

What kind of change does this PR introduce?

Feature

What is the current behavior?

package.json "prettier" config is ignored

What is the new behavior?

Prettier config will be, in order of precedence:

  • "prettier" key in package.json, if it exists
  • prettierrc file in the root of the sandbox
  • sandbox prettier config (via preferences)
  • default prettier config

What steps did you take to test this? This is required before we can merge, make sure to test the flow you've updated.

Tested all of the above scenarios

Checklist

  • Documentation
  • Testing
  • Ready to be merged
  • Added myself to contributors table

This addresses only part of issue #1980. I didn't implement all the possible configuration files just to keep it simple.

Another thing I wanted to mention, I notice there's another function to get the prettier config here but I didn't manage to see where's being used. Is it being used at all?

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 10, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 7685d68:

Sandbox Source
Notifications Test Configuration

@lbogdan lbogdan temporarily deployed to pr4874 September 10, 2020 11:18 Inactive
@lbogdan
Copy link
Contributor

lbogdan commented Sep 10, 2020

Build for latest commit 7685d68 failed.

@lbogdan lbogdan temporarily deployed to pr4874 September 10, 2020 11:37 Inactive
@lbogdan lbogdan temporarily deployed to pr4874 September 10, 2020 11:50 Inactive
@SaraVieira
Copy link
Contributor

👋

Thank you so much, I'm on my way to a meeting but will review when I get back home

Question would this be possible?. #1980

@pablopunk
Copy link
Contributor Author

@SaraVieira sure, it will require to parse YAML and TOML and a few more checks for the files existing, but it is possible. I can do them all if that's what we want

@SaraVieira
Copy link
Contributor

SaraVieira commented Sep 10, 2020

We do have a toml one installed already. its called markty-toml and it's used for netlifly configs here:

We do not have a YAML one so I am okay with letting that one go and also the TOML one if it's too hard, it's mostly just for the names you can give the prettier config files :)

Mostly this version: prettier.config.js

@SaraVieira
Copy link
Contributor

Also ping me if you need any help

@pablopunk pablopunk marked this pull request as draft September 11, 2020 23:00
@pablopunk pablopunk changed the title Support prettier config in package.json Support all prettier config files Sep 11, 2020
@lbogdan lbogdan temporarily deployed to pr4874 September 11, 2020 23:16 Inactive
@MichaelDeBoey
Copy link
Contributor

MichaelDeBoey commented Sep 21, 2020

@pablopunk Maybe there's some cosmiconfig logic we can (re-)use? 🤔
It's used by a lot of libraries (like eslint, prettier, ...) to get the config of all the different supported files.

@pablopunk
Copy link
Contributor Author

@MichaelDeBoey I did take a look at that but they look for the files in the filesystem (with node)

@pablopunk
Copy link
Contributor Author

@SaraVieira I wanna resolve a module from a path (get the value of module.exports) right here, what's the convention on codesandbox? I know it uses eval, can I see an example?

@SaraVieira
Copy link
Contributor

Hey!

Gonna ping @CompuIves as he might be able to help you better there

@github-actions
Copy link

github-actions bot commented Mar 8, 2021

This PR has automatically been marked stale because there has been no activity in a while. Please leave a comment if the issue has not been resolved, or if it is not stale for any other reason. After 2 weeks, this issue will automatically be closed, unless a comment is made or the stale label is removed.

@github-actions github-actions bot added the stale label Mar 8, 2021
@github-actions
Copy link

This PR has been automatically closed because there wasn't any activity after the previous notice or the stale label wasn't removed.

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.

None yet

4 participants