Skip to content
This repository has been archived by the owner on Jan 13, 2024. It is now read-only.

Fix isPublic check for licenses array #1140

Merged
merged 2 commits into from
Apr 22, 2021
Merged

Conversation

Hypfer
Copy link
Contributor

@Hypfer Hypfer commented Apr 20, 2021

As it turns out, not including bytecode is broken for modules that have a licenses array in their package.json.

One example of this is utils-merge which is a dependency of express.

Without this check, license will become undefined since there's no type property on that array.
This causes the module to be listed as non-public which means that it doesn't get included as CONTENT
That in turn triggers the exception when building without bytecode.

Anyways, this was not fun to debug, because there's no mention anywhere of this isPublic check.

@Hypfer
Copy link
Contributor Author

Hypfer commented Apr 20, 2021

Actually, this doesn't fix the issue #1139, because the module uid2, which is used in the example there, features no license at all and will therefore always be listed as private.

It doesn't matter for my main project and this still fixes a bug, however I suppose it would make sense to hide the isPublic check behind some kind of flag, so that it's just used by people that actually need it and not breaks foss projects for absolutely no reason

Edit:

As it turns out, there actually IS a flag that can disable that check. Unfortunately, there just doesn't seem to be any documentation about it anywhere.

--public-packages=packagea,packageb will disable the check for these two while --public-packages=* will disable the check altogether.

Copy link
Contributor

@robertsLando robertsLando left a comment

Choose a reason for hiding this comment

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

@Hypfer thanks for this fix, could you also mention this flag in docs please?

@Hypfer
Copy link
Contributor Author

Hypfer commented Apr 21, 2021

Done

Copy link
Contributor

@robertsLando robertsLando left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants