jsr-io / jsr

The open-source package registry for modern JavaScript and TypeScript
https://jsr.io
MIT License
2.29k stars 100 forks source link

bug: bun workspace dependencies not resolved #448

Open juliusmarminge opened 4 months ago

juliusmarminge commented 4 months ago

ref https://github.com/t3-oss/t3-env/pull/227#issuecomment-2078859996

when a package depends on a workspace package, jsr fails to resolve that import (at least when using Bun):

@t3-oss/env-core published successfully, but when going to publish @t3-oss/env-nextjs it fails with the following error:

image

dsherret commented 4 months ago

I looked into this and the reason this fails is because when publishing it resolves @t3-oss/env-core to the other workspace package on the registry, but the registry has no knowledge of the workspace once it gets the tarball for the package.

Most likely, npm workspace specifiers should be changed to a JSR specifier on publish. I opened https://github.com/denoland/deno/issues/23638

As a temporary workaround, I believe adding the following would fix it so that it maps the @t3-oss/env specifier to the jsr package:

--- a/packages/nextjs/jsr.json
+++ b/packages/nextjs/jsr.json
@@ -5,6 +5,9 @@
     ".": "./src/index.ts",
     "./presets": "./src/presets.ts"
   },
+  "imports": {
+    "@t3-oss/env": "jsr:@t3-oss/env@^0.10.3"
+  },
   "include": ["src/**/*.ts", "README.md", "LICENSE", "package.json"],
   "exclude": ["node_modules", "dist", "test"]
 }
juliusmarminge commented 3 months ago

Just tried this and it doesn't work:

CleanShot 2024-05-17 at 12 01 33

MeowningMaster commented 3 months ago

I managed to publish my library on JSR from bun monorepo https://github.com/MeowningMaster/wsx https://jsr.io/@wsx

andeeplus commented 2 months ago

Same issue here, any update on this? I've tried several possible solution but no one helped. @MeowningMaster I've checked you repo and I found some nice tricks in order to address some of the problems I had, thanks for posting it. I think the reason why is working on your side is that your shared package do not have external deps.

dsherret commented 2 months ago

Yeah, it's getting closer. As of two days ago we understand npm workspaces so we can identify the other packages in the workspace (this will be in the next Deno minor release next week). An issue in this situation is we don't yet support wildcards in npm workspaces (ex. "workspaces": ["packages/*"]). Follow this issue for updates: https://github.com/denoland/deno/issues/24420

dsherret commented 1 month ago

This is now working for me with no changes to the original branch when publishing from the workspace root:

> npx jsr publish --dry-run
Simulating publish of @t3-oss/env-core@0.10.3 with files:
   file:///V:/t3-env/packages/core/CHANGELOG.md (7.57KB)
   file:///V:/t3-env/packages/core/README.md (872B)
   file:///V:/t3-env/packages/core/jsr.json (256B)
   file:///V:/t3-env/packages/core/package.json (998B)
   file:///V:/t3-env/packages/core/src/index.ts (11.32KB)
   file:///V:/t3-env/packages/core/src/presets.ts (2.66KB)
   file:///V:/t3-env/packages/core/tsconfig.json (92B)
Simulating publish of @t3-oss/env-nuxt@0.10.1 with files:
   file:///V:/t3-env/packages/nuxt/CHANGELOG.md (7.17KB)
   file:///V:/t3-env/packages/nuxt/README.md (831B)
   file:///V:/t3-env/packages/nuxt/jsr.json (256B)
   file:///V:/t3-env/packages/nuxt/package.json (1.11KB)
   file:///V:/t3-env/packages/nuxt/src/index.ts (1.67KB)
   file:///V:/t3-env/packages/nuxt/src/presets.ts (262B)
   file:///V:/t3-env/packages/nuxt/tsconfig.json (92B)
Simulating publish of @t3-oss/env-nextjs@0.10.1 with files:
   file:///V:/t3-env/packages/nextjs/CHANGELOG.md (9.36KB)
   file:///V:/t3-env/packages/nextjs/README.md (1.45KB)
   file:///V:/t3-env/packages/nextjs/jsr.json (258B)
   file:///V:/t3-env/packages/nextjs/package.json (1.11KB)
   file:///V:/t3-env/packages/nextjs/src/index.ts (2.78KB)
   file:///V:/t3-env/packages/nextjs/src/presets.ts (262B)
   file:///V:/t3-env/packages/nextjs/tsconfig.json (92B)
Warning Aborting due to --dry-run

Completed in 231ms

And with --log-level=debug it seems to be resolving everything correctly.

andeeplus commented 1 month ago

@dsherret Thanks for the update.

I've tried it on my project too. Now the pipeline runs smoothly and all packages are published correctly but unfortunately the imports for the workspace packages in TypeScript files are incorrectly pointing to the package using the jsr specifier.

For instance, you can see an example of this issue here

Additionally, I've noticed that dependencies marked with workspace:* are being removed from the package.json file generated by jsr for the npm layer. Despite attempting to configure the workspace packages in the jsr.json as imports, the issue persists.

In my understanding, shouldn't the workspace:* dependencies be replaced with the current version of the package instead of being removed? Does it make sense for non-deno project configuring the imports for the workspace packages in jsr.json? I thought this was the key to map deps to the package but maybe I was wrong.

I appreciate your guidance on this matter.

Thank you very much!

dsherret commented 1 month ago

I've tried it on my project too. Now the pipeline runs smoothly and all packages are published correctly but unfortunately the imports for the workspace packages in TypeScript files are incorrectly pointing to the package using the jsr specifier.

Isn't that correct for them to be referencing the other JSR package? What's the output you expected?

Additionally, I've noticed that dependencies marked with workspace:* are being removed from the package.json file generated by jsr for the npm layer. Despite attempting to configure the workspace packages in the jsr.json as imports, the issue persists.

I think some of those dependencies aren't listed because they're not imported by any export? Do you have an example of the package not working? I just tried importing every export of that package and it seems to work for me.

andeeplus commented 1 month ago

Thanks for the quick response.

For instance, let's pick the package I sent you above as an example @ecopages/core

If you check the import at line 1:

import { PostCssProcessor } from 'jsr:@ecopages/postcss-processor@^0.1.8';

This is not valid outside of Deno, I'd expect the following:

import { PostCssProcessor } from '@ecopages/postcss-processor';

If you check the package.json created by the npm layer for @ecopages/core, the dependencies that are imported as 'workspace:*' are not listed, and they are consequently not installed either.

So in my package @ecopages/core there is no trace of @ecopages/postcss-processor. Imports in ts files are importing a wrong ref and the dependencies list that should include it, it does not.

About @ecopages/postcss-processor it lists the exports in jsr.json.

I hope I have made myself clear, Thanks a lot

andeeplus commented 1 month ago

I think some of those dependencies aren't listed because they're not imported by any export? Do you have an example of the package not working?

Ah wait a second, do you mean that since no one file that has been exported is using it, this is the reason why it is not listed it in the dependencies?

Looks like this logic can break this and other packages, I need all the dependencies listed in the package json also if the file that I'm exporting are not using it. I'm exporting just the dependencies helpful for the user, not everything, the stuff I'm not exporting is needed too. I think this is the correct behaviour, am I missing something?

I believe I'm now understanding the root cause of the issues with my package, which likely stems from this ticket and the oversight of binary files by JSR.

https://github.com/jsr-io/jsr/issues/636#issuecomment-2213032600

Since the bin script it is not added to the node_modules .bin folder, I was trying to import directly the the ts file from the source, but this is not possible to be used outside of Deno, due the way jsr use to compile the library for the npm layer. So basically ts files are useless in an environment different, from Deno. Am I correct?

Upon examining another package that utilizes the same dependency, it is correctly listed in the package.json dependencies within the npm layer. Additionally, while the JSR documentation appears to reference an incorrect file, the import path is accurate in the npm layer.

JSR docs import { PostCssProcessor } from 'jsr:@ecopages/postcss-processor@^0.1.8';

NPM layer / node_modules import { PostCssProcessor } from '@jsr/ecopages__postcss-processor';

{
  "name": "@jsr/ecopages__bun-postcss-loader",
  "version": "0.1.8",
  "homepage": "https://jsr.io/@ecopages/bun-postcss-loader",
  "type": "module",
  "dependencies": {
    "@jsr/ecopages__postcss-processor": "^0.1.8"
  },
  "exports": {
    ".": {
      "types": "./_dist/src/bun-postcss-loader.d.ts",
      "default": "./src/bun-postcss-loader.js"
    }
  },
  "_jsr_revision": 11
}
andeeplus commented 1 month ago

@dsherret, confirming that exporting the script intended for the node_modules/.bin directory resolved the problem with missing dependencies.

The main drawback is the necessity to directly reference and execute the script located in the node_modules folder, rather than using a simpler, single-word command in the package.json scripts. But the lib is usable, finally.

For reference, the issue related to the .bin directory can be tracked on this ticket: https://github.com/jsr-io/jsr/issues/157

Regarding the limitation of ts files being compatible only with Deno, it seems this will remain the case until other runtimes adopt the prefix:convention as outlined here: https://jsr.io/docs/native-imports. (EDIT: Found this ticket https://github.com/jsr-io/jsr/issues/640)

Thanks a lot