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

Fix parsing weird formats of the keywords field of some package.json files #161

Merged
merged 3 commits into from Dec 12, 2021

Conversation

01walid
Copy link
Contributor

@01walid 01walid commented Dec 9, 2021

Latest version of turborepo was having issues parsing lodash's package.json file. Because it has the keywords field defined as:

"keywords": "modules, stdlib, util"

And not using the standard format:

"keywords": ["modules", "stdlib", "util"]
  • lodash version used is: v4.17.21 (latest version).

Turborepo was failing (exiting) with errors like:

 ERROR  error parsing ... /node_modules/lodash/package.json: json: cannot unmarshal string into Go struct field PackageJSON.keywords of type []string

This PR handles the above and some slightly weirder ways of defining keywords in package.json

I incremented the version, LMK if this is not the scope of this PR.

@vercel
Copy link

vercel bot commented Dec 9, 2021

@01walid is attempting to deploy a commit to the Vercel Team on Vercel.

A member of the Team first needs to authorize it.

@jaredpalmer
Copy link
Contributor

Thanks! This looks awesome. If you can unwind the version bump then I can merge.

@01walid 01walid force-pushed the fix-parsing-keywords-in-package-json branch from 8c43162 to 0886264 Compare December 10, 2021 08:09
@01walid
Copy link
Contributor Author

01walid commented Dec 10, 2021

@jaredpalmer done! rebased off the main branch.

@01walid 01walid force-pushed the fix-parsing-keywords-in-package-json branch from 0886264 to 1e9af57 Compare December 10, 2021 08:36
@jaredpalmer
Copy link
Contributor

This is definitely a bug, but also my guess is you have a double star /** glob in your workspaces declaration in package.json and turbo is scanning your node_modules by accident. This is a bug in turbo. However, in the meantime, only use single stars /* in workspaces

@jaredpalmer
Copy link
Contributor

Can you actually remove Keywords, Description, Homepage, License, Files, and Main from the struct? We shouldn't even waste time on these keys we aren't using.

CleanShot 2021-12-10 at 10 42 54@2x

@01walid
Copy link
Contributor Author

01walid commented Dec 10, 2021

This is definitely a bug, but also my guess is you have a double star /** glob in your workspaces declaration in package.json and turbo is scanning your node_modules by accident. This is a bug in turbo. However, in the meantime, only use single stars /* in workspaces

@jaredpalmer Interesting, we indeed use double star, I'll run some tests with /*.

Does this apply to exclude rules? e.g. !**/test/**?

Can you actually remove Keywords, Description, Homepage, License, Files, and Main from the struct? We shouldn't even waste time on these keys we aren't using.

CleanShot 2021-12-10 at 10 42 54@2x

Exactly my initial thought, I noticed Keywords isn't used anywhere in the codebase, but thought it might be the case in the future. Will do this.

@vercel
Copy link

vercel bot commented Dec 10, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/vercel/turbo-site/5mFmPi46yZdQRHRj3n5jZQckoGyw
✅ Preview: https://turbo-site-git-fork-01walid-fix-parsing-keywords-in-pack-3308f9.vercel.sh

@01walid
Copy link
Contributor Author

01walid commented Dec 12, 2021

@jaredpalmer I went ahead with removing the fields you mentioned.

@jaredpalmer jaredpalmer merged commit 398521d into vercel:main Dec 12, 2021
sokra added a commit that referenced this pull request Oct 25, 2022
sokra pushed a commit that referenced this pull request Oct 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support doublestar glob directives in workspaces arrays
2 participants