Skip to content

fix: attempt to fix module consumption for ESM, TS+ESM, CJS and TS+CJS#12

Open
0gust1 wants to merge 7 commits intorcsb:masterfrom
mabsilico:fix/esm_def
Open

fix: attempt to fix module consumption for ESM, TS+ESM, CJS and TS+CJS#12
0gust1 wants to merge 7 commits intorcsb:masterfrom
mabsilico:fix/esm_def

Conversation

@0gust1
Copy link
Contributor

@0gust1 0gust1 commented Jun 11, 2025

closes #11

  • reworked dual tsconfig: one for ESM and one for commonJS, with common part. tsconfig files may also now be a bit clearer/smaller.
  • additional package.json fields for exposing modules and types for both targets (esm & cjs), with graceful degradation for older envs/contexts
  • build adjustments, including copy of type definitions from lib/ to build/ (for the sake of clarity for package consumers)
  • workaround for lzutf8 (this one looks a bit uncommon)

Package.json fields:

  • main: default entry point when using commonJS. Fallback entry point for older Node.js environments or tools that don't support ES modules (backward compat)
  • module: default entry point for ES modules
  • exports: Modern way to define package entry points with conditional exports. Overrides main and module in compatible environments.
    • . - The main entry point when someone imports the package
    • import - Used for ES modules (import statements)
    • require - Used for CommonJS (require() calls)
    • types - TypeScript definitions for this export
    • ./lib/* and ./build/* - Allow deep imports like @rcsb/rcsb-api-tools/lib/something
  • files: specifies which files/directories to include when publishing to npm

The result may deviate a bit from what was written in the changelog of the 5.0.0 version (e.g. generated types are exposed from both lib/ and build/, which may be clearer from the viewpoint of a consumer (IMHO)).

@0gust1 0gust1 marked this pull request as ready for review June 11, 2025 01:54
@sonarqubecloud
Copy link

@0gust1
Copy link
Contributor Author

0gust1 commented Jun 12, 2025

@bioinsilico I reworked the files to have a more readable diff in this PR.

(it introduced "noisy" commits, which I can fixup later in a rebase, if needed)

@bioinsilico
Copy link
Member

@0gust1 Thank you so much for this contribution. I will test the PR with the applications using the API tools and come back to you.

As a scientific software developer, completely biased towards Scientific, it is always a nightmare to understand and deal with all these technical issues.

@0gust1
Copy link
Contributor Author

0gust1 commented Jun 13, 2025

@bioinsilico 🤞

I'm glad to help!

  • my company was able to build interesting features, in part with the help of those graphQL APIs, and all the work behind.
  • at a personal level, I'm very proud to be able to contribute, even a little, to rcsb/open science/commons.

@0gust1
Copy link
Contributor Author

0gust1 commented Jun 13, 2025

@bioinsilico FYI, I also have a local branch where I auto updated the project's dependencies (npm outdated to see the deps, npm update to auto update the dependencies to minor/patch version).

I didn't pushed the resulting package-lock.json file here (to keep changeset small). Maybe a thing for a next PR/package version bump.

@bioinsilico
Copy link
Member

Thank you again for all your help, @0gust1. I have published a dev version (5.2.0-dev-js-pr12.1) in the npm registry. Our packages using this release only needed minor adjustments. Could you test it before merging and publishing as the latest release?

@0gust1
Copy link
Contributor Author

0gust1 commented Jun 13, 2025

Our packages using this release only needed minor adjustments

out of curiosity, which kind of adjustments ?

Could you test it before merging and publishing as the latest release?

Damn. It weirdly doesn't work. I still have the same module resolution error Cannot find module '/<PATH>/rcsb_api_tools_repro/node_modules/@rcsb/rcsb-api-tools/lib/RcsbLocalStorage/LocalStorageTools' imported from /<PATH>/rcsb_api_tools_repro/node_modules/@rcsb/rcsb-api-tools/lib/RcsbGraphQL/GraphQLRequest.js

Why weird ? what I did

use the local @rcsb/rcsb-api-tools copy

Using the minimal project I linked in the issue (https://github.com/0gust1/Sveltekit-RCSB_API_tools-repro)

  • I did a npm run clean && npm run buildApp && npm link in my local copy of @rcsb/rcsb-api-tools (on the branch mabsilico:fix/esm_def)
  • I did npm link @rcsb/rcsb-api-tools && npm run dev in the Sveltekit-RCSB_API_tools-repro project (so the repro project uses my local copy of @rcsb/rcsb-api-tools as a dependency)

=> everything works OK

use the published 5.2.0-dev-js-pr12.1 package

Then I did:
npm i @rcsb/rcsb-api-tools@5.2.0-dev-js-pr12.1 in the Sveltekit-RCSB_API_tools-repro project (so the repro project now uses your published dev version)

=> fails with the error message

@bioinsilico additionally

  • which version of npm (and node) did you use to publish the @rcsb/rcsb-api-tools@5.2.0-dev-js-pr12.1 package ?
  • what is the sequence of commands did you use to publish this 5.2.0-dev-js-pr12.1 version ?

@0gust1
Copy link
Contributor Author

0gust1 commented Jun 13, 2025

Leads and investigation (draft/pending)

The published package tarball seems to lack something that the local version have (despite the code being the same, in principle)... but I don't see any .npmignore file in the project.

hypothesis 1

there is maybe a gotcha: https://docs.npmjs.com/cli/v11/using-npm/developers#keeping-files-out-of-your-package
If there is no .npmignore file, npm will use the content of .gitignore, and then there can be conflicts/mess between the files field of package.json and the ignored paths (I know... it's hair pulling/depressing).
confidence-level: low

hypothesis 2 (ongoing)

I'll try to locally package a tarball and use it directly (instead of npm link), to be able to replicate.
Ongoing, will update later

@bioinsilico
Copy link
Member

bioinsilico commented Jun 16, 2025

I haven’t had the time to really dig into this yet, and honestly, I’m a bit lost.

which version of npm (and node) did you use to publish the @rcsb/rcsb-api-tools@5.2.0-dev-js-pr12.1 package?

(base) joan@joansmacstudio rcsb-api-tools % node -v
v22.14.0
(base) joan@joansmacstudio rcsb-api-tools % npm -v 
10.9.2

what is the sequence of commands did you use to publish this 5.2.0-dev-js-pr12.1 version ?

To publish the package first I run buildApp and then publishApp

I will do some testing with the setup that you provided me and see if I can find something useful.

@0gust1
Copy link
Contributor Author

0gust1 commented Jun 25, 2025

I haven't forgot :)
I'll rework the topic as soon as my priorities at work will enable it.

@bioinsilico
Copy link
Member

Haha, you've already done too much. I am also busy with another project and haven't found the time to delve into this issue.

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

Successfully merging this pull request may close these issues.

Error when attempting to use GraphQLRequest

2 participants