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

fixing the release #1401

Merged
merged 6 commits into from
Jun 1, 2023
Merged

fixing the release #1401

merged 6 commits into from
Jun 1, 2023

Conversation

MelSumner
Copy link
Contributor

@MelSumner MelSumner commented Jun 1, 2023

in #1397 a user has reported that types are missing and they are getting an error.

This PR updates the .npmignore file and the package.json file to more correctly reflect what should be getting published re:types.

This error seems to be related to the differences between npm pack and yarn pack.

Also not 100% sure this will be the fix but thanks to @gitKrystan @chriskrycho and @NullVoxPopuli for working through this toward a potential solution.

@MelSumner MelSumner added the bug label Jun 1, 2023
@MelSumner MelSumner linked an issue Jun 1, 2023 that may be closed by this pull request
@MelSumner MelSumner requested a review from ef4 June 1, 2023 16:16
@MelSumner MelSumner removed the bug label Jun 1, 2023
@MelSumner
Copy link
Contributor Author

Ok it does seem like there are some undocumented pre-publish steps; working on a new release now. This reversion isn't necessary so I'm closing the PR.

@MelSumner MelSumner closed this Jun 1, 2023
@MelSumner MelSumner deleted the melsumner/revert-npmignore branch June 1, 2023 17:31
@MelSumner MelSumner restored the melsumner/revert-npmignore branch June 1, 2023 18:19
@MelSumner
Copy link
Contributor Author

so I tried running some of the scripts in the package.json and re-publish, but that didn't seem to fix it either, so looks like we might need to revert the .npmignore change after all.

@MelSumner MelSumner reopened this Jun 1, 2023
@gitKrystan
Copy link
Collaborator

Looking into this. I think we need something like this in package.json:

https://github.com/emberjs/ember.js/blob/87c7fc72a3112936ae701a6768072d31caf4730d/package.json#LL173C3-L182C5

@MelSumner
Copy link
Contributor Author

@gitKrystan so would that replace what's in there now?

"typesVersions": {
    "*": {
      "*": [
        "public-types/@ember/test-helpers/*"
      ]
    }
  },

@NullVoxPopuli
Copy link
Sponsor Collaborator

@gitKrystan
Copy link
Collaborator

Looks like it should be:

  "typesVersions": {
    "*": {
      "*": [
        "public-types/*"
      ]
    }
  }

However, I'm not sure what will force the public-types folder to be included in the published package.

I was able to confirm that 3.0 does not currently include the public-types folder but I'm not able to reproduce that locally (using yalc) so I'm scared to guarantee that the above change will fix everything. I was, however, able to confirm that the typesVersions config was wrong and that the above is correct.

@MelSumner MelSumner changed the title revert npmignore change fixing the release Jun 1, 2023
@MelSumner MelSumner added the bug label Jun 1, 2023
@MelSumner MelSumner marked this pull request as ready for review June 1, 2023 21:27
addon/package.json Outdated Show resolved Hide resolved
@@ -149,7 +149,7 @@
"typesVersions": {
"*": {
"*": [
"public-types/@ember/test-helpers/*"
"./public-types/*"
Copy link
Contributor Author

@MelSumner MelSumner Jun 1, 2023

Choose a reason for hiding this comment

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

@gitKrystan @chriskrycho there was a tiny variation between both of your suggestions, should it be ./public-types/* or public-types/*?

Copy link
Collaborator

@gitKrystan gitKrystan Jun 1, 2023

Choose a reason for hiding this comment

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

Either should work. (Confirmed both of them locally just to be sure.)

@MelSumner
Copy link
Contributor Author

So even with these changes, I'm still not seeing the public_types folder when I run npm pack locally. 🤔

@MelSumner
Copy link
Contributor Author

Changing to dist-types from public-types seems to resolve the issue, in that I now get it locally when I run npm pack to check. But...why? makes no real logical sense. Why would it work before but not now? 🤔

@MelSumner MelSumner merged commit 510dec3 into master Jun 1, 2023
13 checks passed
@MelSumner MelSumner deleted the melsumner/revert-npmignore branch June 1, 2023 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Types missing from version 3
3 participants