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(common,config): remove chalk usage #1824

Merged
merged 4 commits into from
Oct 24, 2023
Merged

fix(common,config): remove chalk usage #1824

merged 4 commits into from
Oct 24, 2023

Conversation

emersonhsieh
Copy link
Contributor

@emersonhsieh emersonhsieh commented Oct 24, 2023

When building a MUD v2 client project with vite (pnpm run build), the chalk dependency in @latticexyz/config and @latticexyz/common causes the following error:

error during build:
Error: [vite]: Rollup failed to resolve import "#ansi-styles" from "../../node_modules/.pnpm/[email protected]/node_modules/chalk/source/index.js".
This is most likely unintended because it can break your application at runtime.
If you do want to externalize this module explicitly add it to
`build.rollupOptions.external`
    at onRollupWarning (file:///vercel/path0/node_modules/.pnpm/[email protected]_@[email protected]/node_modules/vite/dist/node/chunks/dep-51c4f80a.js:45522:19)
    at onwarn (file:///vercel/path0/node_modules/.pnpm/[email protected]_@[email protected]/node_modules/vite/dist/node/chunks/dep-51c4f80a.js:45293:13)
    at Object.onwarn (file:///vercel/path0/node_modules/.pnpm/[email protected]/node_modules/rollup/dist/es/shared/rollup.js:23263:13)
    at ModuleLoader.handleResolveId (file:///vercel/path0/node_modules/.pnpm/[email protected]/node_modules/rollup/dist/es/shared/rollup.js:22158:26)
    at file:///vercel/path0/node_modules/.pnpm/[email protected]/node_modules/rollup/dist/es/shared/rollup.js:22119:26
 ELIFECYCLE  Command failed with exit code 1.
Error: Command "pnpm run build" exited with 1

which can be fixed by adding chalk to external dependencies in vite.config.ts:

  build: {
    rollupOptions: {
      external: ["chalk"], // fix for chalk
        ...
      },
    },
    target: "ES2022",
  },

but this results in the following runtime error in the browser as shown in the screenshot. Note that this is a production build served by serve dist:

Uncaught TypeError: Failed to resolve module specifier "chalk". Relative references must start with either "/", "./", or "../".

image

Upon inspecting our bundle, @latticexyz/config which imports chalk appears in our browser bundle:

image

image

This PR removes the dependency on chalk so that if it's imported by any submodules that shares a pnpm-lock.yaml with a browser client, chalk would not need to be externalized or imported. Our client also uses getBurnerPrivateKey and imports from @latticexyz/common/chains which directly requires @latticexyz/common.

Discord thread

@changeset-bot
Copy link

changeset-bot bot commented Oct 24, 2023

🦋 Changeset detected

Latest commit: 523b8e8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 30 packages
Name Type
@latticexyz/common Patch
@latticexyz/config Patch
@latticexyz/block-logs-stream Patch
@latticexyz/cli Patch
@latticexyz/dev-tools Patch
@latticexyz/protocol-parser Patch
@latticexyz/store-indexer Patch
@latticexyz/store-sync Patch
@latticexyz/store Patch
@latticexyz/world-modules Patch
@latticexyz/world Patch
@latticexyz/react Patch
@latticexyz/abi-ts Patch
create-mud Patch
@latticexyz/ecs-browser Patch
@latticexyz/faucet Patch
@latticexyz/gas-report Patch
@latticexyz/network Patch
@latticexyz/noise Patch
@latticexyz/phaserx Patch
@latticexyz/recs Patch
@latticexyz/schema-type Patch
@latticexyz/services Patch
@latticexyz/solecs Patch
solhint-config-mud Patch
solhint-plugin-mud Patch
@latticexyz/std-client Patch
@latticexyz/std-contracts Patch
@latticexyz/store-cache Patch
@latticexyz/utils Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@holic holic changed the title Remove chalk dependency from @latticexyz/config and @latticexyz/common fix(common,cli): remove chalk usage Oct 24, 2023
@holic holic changed the title fix(common,cli): remove chalk usage fix(common,config): remove chalk usage Oct 24, 2023
@holic holic merged commit 3e05706 into latticexyz:main Oct 24, 2023
9 checks passed
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.

2 participants