Skip to content

Added more oxfmt settings - #2

Open
mrjono1 wants to merge 5 commits into
mainfrom
oxfmt-suggestions
Open

Added more oxfmt settings#2
mrjono1 wants to merge 5 commits into
mainfrom
oxfmt-suggestions

Conversation

@mrjono1

@mrjono1 mrjono1 commented Jul 14, 2026

Copy link
Copy Markdown

Settings added to cover some things that eslint did that oxfmt does

  • Created .vscode/extensions.json to recommend the oxc-vscode extension.
  • Added .vscode/settings.json to configure default formatter and formatting options.
  • Refactored code in sample.ts, base.ts, react.ts, shared.ts, and index.ts for consistent formatting.
  • Updated oxfmt configuration with new formatting options and overrides for Markdown files.
  • Adjusted test file patterns in vitest/base.ts for improved clarity.

- Created `.vscode/extensions.json` to recommend the `oxc-vscode` extension.
- Added `.vscode/settings.json` to configure default formatter and formatting options.
- Refactored code in `sample.ts`, `base.ts`, `react.ts`, `shared.ts`, and `index.ts` for consistent formatting.
- Updated `oxfmt` configuration with new formatting options and overrides for Markdown files.
- Adjusted test file patterns in `vitest/base.ts` for improved clarity.
Comment thread .vscode/settings.json Outdated
Comment thread src/oxfmt/index.ts

@samchungy samchungy Jul 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This one was migrated using the oxfmt --migrate command against the base skuba config, what's all the other stuff?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

ill add inline comments

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

There is intellisense on every property

Comment thread src/oxfmt/index.ts Outdated
Comment thread src/oxfmt/index.ts
tabWidth: 2,
trailingComma: 'all',
printWidth: 80,
endOfLine: 'lf',

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

ensure we don't have a mix of crlf and lf

Comment thread src/oxfmt/index.ts

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

There is intellisense on every property

Comment thread src/oxfmt/index.ts
Comment on lines +15 to +20
sortImports: {
groups: ['builtin', 'external', ['internal', 'subpath'], ['parent', 'sibling', 'index'], 'unknown'],
newlinesBetween: true,
ignoreCase: false,
order: 'asc',
},

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Eslint raised an issue on this but here we just do the formatting for you

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Interesting - in the past I think a sort import rule would handle this for you in ESLint via the eslint-import-x-plugin.

Is it better to use the import plugin or this?

image

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

oxc look to have moved this to more of a formatting concern

Comment thread src/oxfmt/index.ts
arrowParens: 'always',
bracketSpacing: true,
quoteProps: 'as-needed',
proseWrap: 'always',

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

propose word wrapping

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am not sure this should be added as a part of the first release, just in the name of consistency.

Also seems it only applies to Markdown, MDX and YAML 🤔
https://oxc.rs/docs/guide/usage/formatter/config-file-reference.html#prosewrap

We've already removed Markdown from that list and as someone who owns too much mdx I am not sure we'd want it on that either... and at that point is it worth it just for yaml 🤷

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

commented in other line, yes this should not be for md or yaml

Comment thread src/oxfmt/index.ts
Comment on lines +23 to +26
files: ['*.md'],
options: {
proseWrap: 'preserve',
},

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

don't do word wrapping in markdown as that breaks the markdown

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@zbrydon this ensures that it wont affect markdown, we can add yaml and yml here as well

Comment thread src/oxfmt/index.ts Outdated

@zbrydon zbrydon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'll have to find some time to have a proper look but initially my thoughts on the direction we should take is attempt to replicate the prettier and eslint behaviour exactly introducing no changes that we don't have to then where there are inherent differences we come to a consensus on what direction we want to go with.

From there we can start adding any/all the new fancy settings as we se fit in such a away that is easier to digest than a migration and rule changes for consumers

Comment thread src/oxfmt/index.ts
arrowParens: 'always',
bracketSpacing: true,
quoteProps: 'as-needed',
proseWrap: 'always',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am not sure this should be added as a part of the first release, just in the name of consistency.

Also seems it only applies to Markdown, MDX and YAML 🤔
https://oxc.rs/docs/guide/usage/formatter/config-file-reference.html#prosewrap

We've already removed Markdown from that list and as someone who owns too much mdx I am not sure we'd want it on that either... and at that point is it worth it just for yaml 🤷

Comment thread src/oxfmt/index.ts
Comment on lines +10 to +13
semi: true,
arrowParens: 'always',
bracketSpacing: true,
quoteProps: 'as-needed',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What are our thoughts on explicitly setting defaults?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

we can remove most but bracketSpacing: true, does come up a lot, it would be great to keep just this one

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.

3 participants