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

Improves JS constraints #5341

Merged
merged 4 commits into from
Mar 27, 2023
Merged

Improves JS constraints #5341

merged 4 commits into from
Mar 27, 2023

Conversation

arcanis
Copy link
Member

@arcanis arcanis commented Mar 24, 2023

What's the problem this PR addresses?

  • It's difficult to use JS constraints because they're not covered by types.

  • The automatic constraints check occurs before install; this has a couple of critical issues:

    • If a constraint fails because of yarn add, Yarn will request to call yarn constraints for details but won't persist the changes on disk. As a result, yarn constraints will remain silent.
    • If a constraint has to be fixed by calling multiple commands (for example to add a dependency in multiple places), users currently have to manually edit the package.json (because the yarn add / yarn up changes aren't persisted on disk).
    • If the yarn.config.js file call require('./.pnp.cjs') (a bit experimental at the moment, but why not), it causes problems during merge conflict resolution (the merge conflicts in the .pnp.cjs file aren't resolved until after the install finishes).

How did you fix it?

  • Created a types package @yarnpkg/types - it's meant to only contain "dumb" types, and only contains the constraints context at the moment. Later on, I plan to expand it to also cover plugins, but that'll be a separate project.

  • The automated constraints checks will now occur after install rather than before.

Checklist

  • I have set the packages that need to be released for my changes to be effective.
  • I will check that all automated PR checks pass before the PR gets reviewed.

@RDIL
Copy link
Member

RDIL commented Mar 24, 2023

🎉

@arcanis arcanis merged commit cd27d2a into master Mar 27, 2023
@arcanis arcanis deleted the mael/constraints-check-types branch March 27, 2023 11:01
@koshic
Copy link

koshic commented Mar 27, 2023

@arcanis thx for this PR! Could you please share your plans about this feature?

As an example, for my projects it still doesn't work due to 'type: module' in root package.json and ERR_REQUIRE_ESM fail. Minimal requirement is to support .cjs (instead of hardcoded 'ppath.join(this.cwd, yarn.config.js)'), but existing 'constraintsPath' option should be enough too.

@arcanis
Copy link
Member Author

arcanis commented Mar 27, 2023

I think it'd be reasonable to change the logic to yarn.config.cjs, since it's already the extension we use for .pnp.cjs 🤔

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.

3 participants