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: fix typo in postcssPlugin #764

Merged
merged 1 commit into from
Nov 14, 2022
Merged

fix: fix typo in postcssPlugin #764

merged 1 commit into from
Nov 14, 2022

Conversation

gavinsharp
Copy link
Contributor

When building with tsup I have configured esbuild to use the new copy loader for CSS files:

const { build } = require('tsup');
build({
  // ...
  loader: {
  	'.css': 'copy'
  },
});

Unfortunately this is causing an error when building due to an old version of postcss I have in my dependency tree:

✘ [ERROR] [plugin postcss] postcss.default is not a function

    ../../utilities/module-scripts/node_modules/tsup/dist/index.js:1348:74:
      1348 │           const result = await (postcss == null ? void 0 : postcss.default(plugins).process(contents, { ...options, from: args.path }));

I was trying to figure out why tsup was trying to invoke postcss at all, and discovered what looks like a typo, which causes invocation of postcss even if there is no postcss config present.

One thing I am not sure about: I feel like given my config we should not be including the esbuild postcssPlugin at all? So maybe this should also be fixed at a higher level? But this was sufficient to avoid the error.

When building with tsup I have configured esbuild to use the new [`copy` loader](https://esbuild.github.io/content-types/#copy) for CSS files:
```
const { build } = require('tsup');
build({
  // ...
  loader: {
  	'.css': 'copy'
  },
});
```

Unfortunately this is causing an error when building due to an old version of `postcss` I have in my dependency tree:
```
✘ [ERROR] [plugin postcss] postcss.default is not a function

    ../../utilities/module-scripts/node_modules/tsup/dist/index.js:1348:74:
      1348 │           const result = await (postcss == null ? void 0 : postcss.default(plugins).process(contents, { ...options, from: args.path }));
```
I was trying to figure out why tsup was trying to invoke postcss at all, and discovered what looks like a typo, which causes invocation of postcss even if there is no postcss config present.

One thing I am not sure about: I feel like given my config we should not be including the esbuild `postcssPlugin` at all? So maybe this should also be fixed at a higher level? But this was sufficient to avoid the error.
@vercel
Copy link

vercel bot commented Nov 6, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
tsup ✅ Ready (Inspect) Visit Preview Nov 6, 2022 at 2:21PM (UTC)

@gavinsharp
Copy link
Contributor Author

gavinsharp commented Nov 6, 2022

One thing I am not sure about: I feel like given my config we should not be including the esbuild postcssPlugin at all? So maybe this should also be fixed at a higher level? But this was sufficient to avoid the error.

I was thinking about e.g. a way to configure tsup and tell it not to do anything CSS-related at all. I guess with injectStyle: false and no postcss config the postcssPlugin should be pretty much a no-op after this PR and #744, though the onLoad hook does still do a readFile, which may be slower than not having an onLoad hook at all?

@egoist egoist changed the title fix typo in postcssPlugin fix: fix typo in postcssPlugin Nov 14, 2022
@egoist egoist merged commit a2803dd into egoist:dev Nov 14, 2022
@github-actions
Copy link
Contributor

🎉 This PR is included in version 6.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

amitdahan pushed a commit to amitdahan/tsup that referenced this pull request Dec 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants