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

Invalid package.json "type" field on npm install #1219

Closed
imathews opened this issue Oct 6, 2022 · 5 comments
Closed

Invalid package.json "type" field on npm install #1219

imathews opened this issue Oct 6, 2022 · 5 comments
Assignees

Comments

@imathews
Copy link

imathews commented Oct 6, 2022

Describe the bug

When installing @h5web/app and @h5web/wasm (@6.3.0), the package.json file for the installed packages in node_modules contains an invalid "type": "" field. The type in package.json should be one of commonjs, module. This is specifically happening for @h5web/lib, @h5web/app, @h5web/h5wasm.

I can see that the package.json in this repo doesn't have any "type" field, so I imagine it must be getting added during a build step.

The incorrect field creates distracting warnings on certain build tools, such as esbuild (see below). I imagine this might effect tree shaking and other module resolution behavior for these tools as well (though FWIW, h5web is working smoothly despite these warnings).

▲ [WARNING] "" is not a valid value for the "type" field [package.json]

    node_modules/@h5web/app/package.json:11:10:
      11 │   "type": "",
         ╵           ~~

  The "type" field must be set to either "commonjs" or "module".

To Reproduce

npm install @h5web/app

Expected behaviour

The "type" field in package.json I believe should be set to "module" for this library

Context

  • OS: macos
  • NPM version: 8.5.5
  • Version: 6.3.0
  • H5Web context: @h5web/lib, @h5web/app, @h5web/h5wasm
@axelboc
Copy link
Contributor

axelboc commented Oct 7, 2022

Hi @imathews, thanks for the report.

Each package has its own package.json with "type": "module": for development. We override this to "type": "" on publish via pnpm's "publishConfig" object.

At first, we used to publish with "type": "module", but then CRA v5 / webpack v5 came around and we had some issues. I can't quite recall what they were -- maybe something with missing js extensions in import paths of dependencies... Yeah looks like it: #1000. But then CRA did not like "commonjs" either for some reason and we removed it: 50c9ef8.

I'll investigate again if the original issue can be resolved another way.

@imathews
Copy link
Author

imathews commented Oct 9, 2022

Thanks for the update @axelboc - all makes sense. I'm not particularly familiar with pnpm, though I wonder if there's a way to just remove the type field entirely from the distributed package.json... perhaps that would be enough to make all these tools happy? But agree if there's a way to sort #1000 in another way, that probably seems like the best path forward.

@axelboc axelboc self-assigned this Oct 14, 2022
@axelboc
Copy link
Contributor

axelboc commented Oct 18, 2022

I've gone with removing the type field entirely as you suggested, in the CI, just before publishing. b6eb771

Can you please try version 6.4.2-beta.1 and let us know if the issue is resolved?

@imathews
Copy link
Author

Just tried this out — this release seems to do the trick (no more warnings on build, and otherwise h5web continues to work great). Thanks @axelboc!

@axelboc
Copy link
Contributor

axelboc commented Oct 19, 2022

Great to hear! 🎉

@axelboc axelboc closed this as completed Oct 19, 2022
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

No branches or pull requests

2 participants