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

[bug] compile @fluentui/react fail after using esbuild-plugin-postcss2 #1834

Closed
baurine opened this issue Dec 7, 2021 · 4 comments
Closed

Comments

@baurine
Copy link

baurine commented Dec 7, 2021

Reproduce steps

  1. Initial a react project by npx esbuild-create-react-app my-app, choose TypeScript template. esbuild-create-react-app

  2. Install @fluentui/react by running yarn add @fluentui/react

  3. Test @fluentui/react components:

    // src/App.tsx
    import { DatePicker, PrimaryButton } from '@fluentui/react'
    
    export default function App() {
      return (
        <div>
          Hello World!
          <PrimaryButton>Hello Fluent UI</PrimaryButton>
          <DatePicker />
        </div>
      )
    }
  4. Running yarn build or yarn start, works fine.

    image

    Demo code: miscs-test/esbuild-demo@d6eb07c

  5. Install esbuild-plugin-postcss2 to help comiple .less CSS, but it tasks over .css/.scss/.styl as well. Running yarn add -D esbuild-plugin-postcss2.

  6. Config esbuild options:

    const postCssPlugin = require('esbuild-plugin-postcss2')
    
    const buildParams = {
        // ...
        plugins: [postCssPlugin.default({})]
    }
  7. Running yarn build or yarn start, report the following errors:

    > node_modules/@fluentui/react/lib/components/ExtendedPicker/PeoplePicker/ExtendedPeoplePicker.js:2:7: error: [plugin: postcss2] ENOENT: no such file or directory, open '/mnt/bao/codes/personal/try-esbuild/esbuild-demo/node_modules/@fluentui/react/lib/components/ExtendedPicker/PeoplePicker/ExtendedPeoplePicker.scss'
        2 │ import './ExtendedPeoplePicker.scss';
          ╵        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      node_modules/esbuild-plugin-postcss2/dist/index.js:65:10: note: This error came from the "onResolve" callback registered here
        65 │     build.onResolve({filter: /.\.(css|sass|scss|less|styl)$/}, async (args) => {
          ╵           ~~~~~~~~~
        at setup (/mnt/bao/codes/personal/try-esbuild/esbuild-demo/node_modules/esbuild-plugin-postcss2/dist/index.js:65:11)
    

    After investigating, it is true that 'node_modules/@fluentui/react/lib/components/ExtendedPicker/PeoplePicker/ExtendedPeoplePicker.scss' doesn't exist, but exist ExtendedPeoplePicker.scss.js.

    image

    So, although this error is thrown by esbuild-plugin-postcss2, I think the root reason comes from esbuild self. It doesn't check whether exist the .js file before passing it to the esbuild-plguin-postcss2 plugin.

    Demo code: miscs-test/esbuild-demo@1a057d9

Workaround

I work around this issue by hacking the esbuild-plugin-postcss2 code in the node_modules, I add the logic to check if it exists the corresponding .js file, just return it back to esbuild self to handle it.

        if (!sourceFullPath)
          sourceFullPath = path.resolve(args.resolveDir, args.path);

        // hack
        let exist = existsSync(sourceFullPath + ".js");
        if (exist) {
          return;
        }
@evanw
Copy link
Owner

evanw commented Dec 7, 2021

This is by design. When plugins take control of module resolution, they are in control. If they are doing the wrong thing then that's a problem with the plugin, not with esbuild, and the plugin should be changed to avoid the issue. Arguably that package could also be changed to not use such problematic file names.

@baurine
Copy link
Author

baurine commented Dec 8, 2021

hi @evanw , thanks for your reply.

I agree with you that if @fluentui/react doesn't use the problematic file names that would be much better, but it is reasonable behavior, when we import a file, we can omit the .ts/.tsx/.js/.jsx suffix.

If we let the plugins do this check, every plugin needs to implement the check ideally, that doesn't make sense very much.

And I believe the esbuild does this check (in this case it means resolve the xx.scss to xx.scss.js correctly) because it can compile the @fluentui/react successfully when we don't install the esbuild-plugin-postcss2. I think we just need to move the check logic before calling plugins. If we do so, esbuild knows xx.scss is xx.scss.js, so it will compile it by esbuild self, won't call the esbuild-plugin-postcss2 instead.

Not sure whether I explain it clearly.

fragsalat added a commit to fragsalat/esbuild-style-plugin that referenced this issue May 2, 2022
Currently it's not possible to compile @fluentui/react package with any style plugin. From the esbuild thread evanw/esbuild#1834 it is mentioned the plugins should take care of properly selecting the files to compile. Thus I added 3 simple lines of code which will prevent inclusion of those precompiled files. In the example of fluentui there is for instance the import for BasePicker.less which indeed exists but it will instead picked the pre-compiled BasePicker.less.js.
Every esbuild style plugin tries to compile these JS files with less or what ever. So the condition just makes sure the resolved file is really a style.
@fragsalat
Copy link

Just wanted to line out that with the above merged pr the esbuild-style-plugin works like a charm with fluentui. It also supports css modules and sass/less/stylus etc and is working for me as I would like to have it.

const esbuild = require('esbuild');
const stylePlugin = require('esbuild-style-plugin');
const less = require('postcss-less');

esbuild.build({
  logLevel: 'info',
  bundle: true,
  entryPoints: ['src/index.tsx'],
  outdir: 'dist',
  target: 'es2020',
  loader: {
    '.js': 'jsx'
  },
  plugins: [
    stylePlugin({
      postcss: {parser: less},
      cssModulesOptions: {
        getJSON: () => { },
        generateScopedName: "[name]__[local]___[hash:base64:5]"
      }
    })
  ]
})

@evanw
Copy link
Owner

evanw commented Dec 4, 2022

After the original comment for this issue was posted, esbuild exposed a resolve API to plugins that I believe could be used by this plugin to handle this case: https://github.com/evanw/esbuild/releases/v0.14.8. So I'm closing this issue because I believe that it's now relatively straightforward for plugins to handle this correctly.

@evanw evanw closed this as completed Dec 4, 2022
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

No branches or pull requests

3 participants