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

feat: add support for TypeScript based configs #632

Merged
merged 8 commits into from
Apr 3, 2023
Merged

feat: add support for TypeScript based configs #632

merged 8 commits into from
Apr 3, 2023

Conversation

xeho91
Copy link
Contributor

@xeho91 xeho91 commented Apr 2, 2023

Resolves #631

This PR contains a:

  • bugfix
  • new feature
  • code refactor
  • test update
  • typo fix
  • metadata update

Motivation / Use-Case

TailwindCSS since version ^3.3.0 has support for TypeScript based configs. Including PostCSS.

Hence, I've added patterns to search for config files, which end with the following extensions:

  • cts
  • mts
  • ts

Breaking Changes

None so far.

Additional Info

#631

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Apr 2, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: xeho91 / name: Mateusz Kadlubowski (dd2166b)

return {
parser: 'sugarss',
syntax: 'sugarss',
// FIXME: To confirm. Apparently there's no longer an available option `mode`?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need help with this one.

I am not sure whether this should be ignored or removed. This config key doesn't exist anymore.

return {
parser: 'sugarss',
syntax: 'sugarss',
// FIXME: To confirm. Apparently there's no longer an available option `mode`?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Is it not avaliable only in types?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it not avaliable only in types?

I am not sure. According to the documentation: https://github.com/postcss/postcss-load-config#options
I don't see the mode option.

I tried searching in their codebase for options, but I can't find a mention of mode either. 🤔

Copy link
Member

Choose a reason for hiding this comment

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

We apply mode to our logic so developer can use the mode option from configuration

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to clarify, this one comes from the webpack configuration - mode?

I guess, in this case, if I am not wrong, I could just merge the types from the PostCSS Config and the webpack config.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@xeho91
Copy link
Contributor Author

xeho91 commented Apr 2, 2023

I had to install two packages:

  • cosmiconfig-typescript-loader to load TypeScript configs, apparently by default cosmiconfig doesn't support TypeScript extensions.
  • postcss-load-config as devDependency, because I needed the types to add to the config. Using any or unknown would break with the current lint setup.

I have added test cases for two config types:

  • postcss.config.ts (ESM)
  • postcss.config.cts (CJS)

Lemme know if it's enough. I have skipped the other possible patterns, e.g. postcssrc.ts, etc.
Locally the tests for config-autoload/ pass successfully for me; however, the other tests fail- and I have no clue if they're relevant. Any guidance would be much appreciated. 🙏

Comment on lines 1 to 4
import type { Config as PostCSSConfig } from 'postcss-load-config';
import { Configuration as WebpackConfig } from 'webpack';

module.exports = function (api: WebpackConfig): PostCSSConfig {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alexander-akait I believe this is the way to solve the issue.


module.exports = function (api: Config) {
module.exports = function (api: WebpackConfig): PostCSSConfig {
Copy link
Member

Choose a reason for hiding this comment

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

Nope, it should be https://github.com/webpack-contrib/postcss-loader/blob/master/src/utils.js#L152, it is mix of types, so I think we just need to add this to documents

Copy link
Member

Choose a reason for hiding this comment

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

Feel free to update types in docs and we can merge + small example, thank you

Add a file for the future reference - `./src/config.d.ts`.
Replaced the old one with a new one, and added a file reference.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alexander-akait I allowed myself to add a new file for the future reference.

I couldn't import type in the test files since this repo doesn't use typescript; hence I have no idea how to load them. So, I've copied the content of this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this what you were hoping to see by updating the docs? @alexander-akait

@alexander-akait
Copy link
Member

Oh, there is a bug in npm, I will merge and will fix it in the other PR, thank you for the PR

@alexander-akait alexander-akait merged commit c6b5def into webpack-contrib:master Apr 3, 2023
@xeho91 xeho91 deleted the feat/support-typescript-config branch April 3, 2023 15:34
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.

Support for TypeScript based configs, eg postcss.config.ts
2 participants