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

test: respect commonjs options in playgrounds #13273

Merged
merged 1 commit into from
May 22, 2023

Conversation

patak-dev
Copy link
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

@stackblitz
Copy link

stackblitz bot commented May 19, 2023

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

@patak-dev patak-dev added the p1-chore Doesn't change code behavior (priority) label May 19, 2023
Copy link
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-dev
Copy link
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-dev patak-dev merged commit 19e8c68 into main May 22, 2023
@patak-dev patak-dev 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