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

require inside of a comment is parsed and treated as an actual import #13831

Closed
7 tasks done
LukasTy opened this issue Jul 14, 2023 · 3 comments
Closed
7 tasks done

require inside of a comment is parsed and treated as an actual import #13831

LukasTy opened this issue Jul 14, 2023 · 3 comments

Comments

@LukasTy
Copy link

LukasTy commented Jul 14, 2023

Describe the bug

Running dev build with @mui/x-date-pickers dependency and using AdapterDayjs produces an error on dev build (production build works without problems).

The following source code is treated as an actual require statement, even though it is inside of a comment:

const localeNotFoundWarning = buildWarning([
  'Your locale has not been found.',
  'Either the locale key is not a supported one. Locales supported by dayjs are available here: https://github.com/iamkun/dayjs/tree/dev/src/locale',
  "Or you forget to import the locale with `require('dayjs/locale/{localeUsed}')`",
  'fallback on English locale',
]);

We can address this issue on the @mui/x side by changing the comment content or the user can simply patch the package in the meantime, but it's still an interesting and inconsistent behavior between dev and production builds.

Reproduction

mui/mui-x#9642 (comment)

Steps to reproduce

  1. Clone the repository linked in the issue comment
  2. Run npm install
  3. Run npm run dev
  4. Observe the thrown error

System Info

System:
    OS: macOS 13.4.1
    CPU: (10) arm64 Apple M1 Pro
    Memory: 444.33 MB / 32.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.16.0 - ~/.nvm/versions/node/v18.16.0/bin/node
    Yarn: 1.22.19 - ~/.nvm/versions/node/v18.16.0/bin/yarn
    npm: 9.5.1 - ~/.nvm/versions/node/v18.16.0/bin/npm
    pnpm: 8.6.7 - ~/.nvm/versions/node/v16.19.0/bin/pnpm
  Browsers:
    Chrome: 114.0.5735.198
    Safari: 16.5.1
  npmPackages:
    @vitejs/plugin-react: ^3.1.0 => 3.1.0 
    vite: ^4.4.3 => 4.4.3

Used Package Manager

npm

Logs

No response

Validations

@sapphi-red
Copy link
Member

This is not what Vite does. This is what vite-plugin-commonjs does. That plugin seems to replace all require calls.
https://github.com/originjs/vite-plugins/blob/568f5b79ba14723c13e5476efeb4cbf28494acd0/packages/vite-plugin-commonjs/src/lib.ts#L3
https://github.com/originjs/vite-plugins/blob/568f5b79ba14723c13e5476efeb4cbf28494acd0/packages/vite-plugin-commonjs/src/lib.ts#L26C30-L29
It seems it doesn't work without the plugin though. I guess it's related to #12423.

@LukasTy
Copy link
Author

LukasTy commented Jul 14, 2023

Thank you for taking the time to look into the root cause of the problem. 🙏
It looks like there are already a few related issues in the plugins repository: originjs/vite-plugins#16, originjs/vite-plugins#27, originjs/vite-plugins#29. 👌

@patak-dev
Copy link
Member

As a workaround we suggest adding a unicode zero-width space (\u200b) for a related issue with import.meta.env.mode. You could use it to avoid the replacement: require\u200b('dayjs/locale/{localeUsed}'), but it wouldn't be advisable if users are meant to copy and paste this part of the message.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants