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

Incompatible with @microsoft/api-extractor (maybe pnpm?). #1513

Closed
rogerleung0411 opened this issue Nov 4, 2021 · 4 comments
Closed

Incompatible with @microsoft/api-extractor (maybe pnpm?). #1513

rogerleung0411 opened this issue Nov 4, 2021 · 4 comments
Labels
bug Something isn't working
Milestone

Comments

@rogerleung0411
Copy link

Describe the bug
image

In my project, i use @microsoft/api-extractor to rollup all d.ts into a single d.ts per package(like what vue 3.0 does).

However, when i migrate yarn to pnpm, and build my project once again, the error(snapshot above) occurs.
It it obvious that api-extractor (code here) tries to read meta info from esm/package.json (or cjs/package.json), while no name field found.

It is neccessary not only provide type field but merge with the root package.json in postcompile.sh?

To Reproduce

  • clone vue next repo
  • access an arbitrary package under /packages/, add socket.io-client to dependencies, then exports * from 'socket.io-client' in index.ts
  • build the package, error will log in terminal.

im not sure it is actually a bug of socket.io-client, or the build tools / package manager tools i used, or even it is not a bug and has walkaround?

@rogerleung0411 rogerleung0411 added the bug Something isn't working label Nov 4, 2021
@rogerleung0411
Copy link
Author

FYI: i confirm that 4.2.0 works fine, and broke since this commit(16b6569) released.

@darrachequesne
Copy link
Member

Hmm, OK, so it seems the parent package.json is not read. I'm wondering whether copying the name would be enough, or if we also need version and other fields.

@rogerleung0411
Copy link
Author

rogerleung0411 commented Nov 5, 2021

I think in this specific case, providing name, version, type is enough ('cause api-extractor mark only name and version required and throw error if field not found. see https://github.com/microsoft/rushstack/blob/master/libraries/node-core-library/src/PackageJsonLookup.ts). But more fields we exposed (targeting to align with root package.json), higher compatibility we have.

The ./package.json field after exports field in https://github.com/socketio/socket.io-client/blob/master/package.json#L20 only works in node 12+ and reading files using syntax below:

// import
import packageJson from 'socket.io-client/package.json';
// or
const packageJson = require('socket.io-client/package.json');

However, in tools like api-extractor, they just tryna read package.json under the same directory(only recrusive when not found) once they encountered an entry file, seems like it's a quite common case.

darrachequesne added a commit to socketio/engine.io-parser that referenced this issue Nov 14, 2021
darrachequesne added a commit to socketio/engine.io-client that referenced this issue Nov 14, 2021
darrachequesne added a commit to socketio/engine.io-client that referenced this issue Nov 14, 2021
darrachequesne added a commit that referenced this issue Nov 16, 2021
Note: the version must be kept in sync when publishing a new release

Related: #1513
darrachequesne added a commit that referenced this issue Jan 4, 2022
The additional package.json file, which was copied to
build/cjs/package.json, did hide the parent one, leading to several
issues and providing no real feature. The other one, copied to
build/esm/package.json, is needed though, to enforce the module type.

Related:

- socketio/socket.io#4194
- #1513
sunrise30 added a commit to sunrise30/socket.io-client that referenced this issue Jan 8, 2022
Note: the version must be kept in sync when publishing a new release

Related: socketio/socket.io-client#1513
sunrise30 added a commit to sunrise30/socket.io-client that referenced this issue Jan 8, 2022
The additional package.json file, which was copied to
build/cjs/package.json, did hide the parent one, leading to several
issues and providing no real feature. The other one, copied to
build/esm/package.json, is needed though, to enforce the module type.

Related:

- socketio/socket.io#4194
- socketio/socket.io-client#1513
@darrachequesne
Copy link
Member

darrachequesne commented Apr 5, 2022

This should be fixed by f56fdd0, included in [email protected].

Please reopen if needed.

@darrachequesne darrachequesne added this to the 4.4.1 milestone Apr 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants