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

feat(pack): support publishConfig.type #4204

Merged
merged 9 commits into from
Mar 29, 2022

Conversation

cometkim
Copy link
Contributor

@cometkim cometkim commented Mar 9, 2022

What's the problem this PR addresses?

This PR allows overriding the "type" field on the manifest when packing a package.

{
  "main": "./src/index.ts",
  "publishConfig": {
    "type": "module",
    "main": "./lib/index.js"
  }
}

I wanna build an ESM-first package, but at the same time, I still wanna implicitly load packages with legacy Node resolution in my dev environment to interop with TypeScript which don't yet fully support Node ESM resolution

...

How did you fix it?

by supporting overriding "type" with "publishConfig" object

...

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.

@cometkim cometkim requested a review from arcanis as a code owner March 9, 2022 20:39
Copy link
Member

@merceyz merceyz left a comment

Choose a reason for hiding this comment

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

packages/gatsby/static/configuration/manifest.json Outdated Show resolved Hide resolved
@cometkim cometkim requested a review from merceyz March 9, 2022 21:20
Copy link
Member

@arcanis arcanis left a comment

Choose a reason for hiding this comment

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

I think the right field is types (or typings, but better only support one), not type 🤔

@merceyz
Copy link
Member

merceyz commented Mar 10, 2022

@arcanis type is the correct field in this case https://nodejs.org/dist/latest-v16.x/docs/api/packages.html#type

@arcanis
Copy link
Member

arcanis commented Mar 10, 2022

Of course, my bad 🤦‍♀️

@cometkim cometkim requested a review from arcanis March 11, 2022 11:16
@merceyz merceyz changed the title Support publishConfig.type feat(pack): support publishConfig.type Mar 22, 2022
@merceyz merceyz enabled auto-merge (squash) March 22, 2022 23:38
@arcanis arcanis disabled auto-merge March 29, 2022 20:23
@arcanis arcanis enabled auto-merge (squash) March 29, 2022 20:23
@arcanis arcanis disabled auto-merge March 29, 2022 20:23
@arcanis arcanis merged commit a235095 into yarnpkg:master Mar 29, 2022
@arcanis
Copy link
Member

arcanis commented Mar 29, 2022

Sorry for the delay @cometkim 🙏

merceyz added a commit that referenced this pull request Feb 16, 2023
* feat: support publishConfig.type

* add version file

* add more assertions for publishConfig test

* fix lint

* fix test

* Update manifest.json

* fix description

* chore: mark cli for release

Co-authored-by: Maël Nison <nison.mael@gmail.com>
Co-authored-by: merceyz <merceyz@users.noreply.github.com>
@cometkim cometkim deleted the publishconfig-type branch April 12, 2023 17:39
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