Skip to content

test: respect commonjs options in playgrounds#13273

Merged
patak-cat merged 1 commit intomainfrom
test/respect-commonjs-options-in-playgrounds
May 22, 2023
Merged

test: respect commonjs options in playgrounds#13273
patak-cat merged 1 commit intomainfrom
test/respect-commonjs-options-in-playgrounds

Conversation

@patak-cat
Copy link
Copy Markdown
Member

Description

Avoid forcing deps optimization for playgrounds that define custom commonjsOptions. When running pnpm run test-build-without-plugin-commonjs, this test: playground/external/__tests__/external.spec.ts was failing and is working now after the PR.

Note: in case you run the tests, we have another test in main failing with this env flag in playground/ssr-resolve that is unrelated to this issue.


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

@bolt-new-by-stackblitz
Copy link
Copy Markdown

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@patak-cat patak-cat added the p1-chore Doesn't change code behavior (priority) label May 19, 2023
Copy link
Copy Markdown
Member

@sapphi-red sapphi-red left a comment

Choose a reason for hiding this comment

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

To make the external playground work with build dep-optimization, it seems we need to add a option to declare whether an external dependency is CJS or ESM.

sapphi-red

This comment was marked as duplicate.

@patak-cat
Copy link
Copy Markdown
Member Author

I wonder if we could tell people to continue using the commonjs plugin for these deps in that case, so we try to avoid this cjs compat features if possible.

@patak-cat patak-cat merged commit 19e8c68 into main May 22, 2023
@patak-cat patak-cat deleted the test/respect-commonjs-options-in-playgrounds branch May 22, 2023 07:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

p1-chore Doesn't change code behavior (priority)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants