swaggerexpert / openapi-path-templating

OpenAPI Path Templating parser, validator and resolver.
Apache License 2.0
2 stars 0 forks source link

Is this a bug `Can't resolve '#apg-lite'` I am using swagger-ui-react #76

Closed kivi closed 3 months ago

kivi commented 4 months ago

I am running a node.js App build on Next.js using Bun.

I think this the first time I am seeing an error, resolving dependencies with #

There are lot of error messages, but all say the same:

Module not found: Can't resolve '#apg-lite'
> 1 | import { identifiers, utilities } from '#apg-lite';
kivi commented 4 months ago

I have also tried with pnpm, so it does not have to do with bun

Removing # from the imports works.

I have just remove # in of `import {...} from '#apg-lite'; in node_modules/openapi-path-templating/*/files-that-error.mjs

And added apg-lite to my dependencies. That's my current workaround and it works.

char0n commented 4 months ago

Hi @kivi,

It's because of this: https://github.com/char0n/openapi-path-templating/blob/main/package.json#L16. It's an imports field supported by Node.js since >=12.20.0. I'm not familiar with Bun, but Next.js@4 is supporting this natively.

image

And added apg-lite to my dependencies.

No need, as this library comes with apg-lite automatically: https://github.com/char0n/openapi-path-templating/blob/main/package.json#L65

kivi commented 4 months ago

I saw that packages.json, that's why I was aksing if this a bug here or some other kind of incompatibility. But I am also not familiar with this kind of declaration in your packages.json and the import with #. However I tried also with pnpm and it wasn't working either with same error message.

What else could I try to check if this a bug or not?

char0n commented 4 months ago

@kivi this is not a bug. This is just lack of support for standard features in your build chain. pnpm, yarn or npm has nothing to do with this. They will successfully install the package:

char0n@mortality-ntb:/tmp $ pnpm install openapi-path-templating
Packages: +2
++
Progress: resolved 2, reused 0, downloaded 2, added 2, done

dependencies:
+ openapi-path-templating 1.5.1

Done in 1.7s
char0n commented 4 months ago

@kivi here is POC repository integrating Next@14 and swagger-ui-react: https://github.com/char0n/swagger-ui-nextjs

Builds without any issue.

kivi commented 4 months ago

Your right, not a bug in this package. I guess it's not my build chain. But some dependencies, that uses this package.

char0n commented 4 months ago

I guess it's not my build chain. But some dependencies, that uses this package.

What do you mean? Not sure if this was ironic or real message ;]

char0n commented 4 months ago

@kivi Bun claim to support both exports and imports package.json fields. More info in: https://bun.sh/docs/runtime/modules

kivi commented 4 months ago

@char0n Wheater your are right with my build chain. Or something else collides with this. I am not doing anything extraordinary, so wondering where the issue could be and everything else works, except accessing this package. And since I was unfamiliar to the import with # and removing this was solving it.

No need here for ironic. The contrary, thanks for your replay and trying to help.

char0n commented 4 months ago

@kivi ok understood.

Did some further investigation and managed to successfully install and run the package:

$ docker run --rm --init --ulimit memlock=-1:-1 -v $(pwd):/app -w /app oven/bun bun add node-gyp
$ docker run --rm --init --ulimit memlock=-1:-1 -v $(pwd):/app -w /app oven/bun bun add swagger-client
$ docker run --rm --init --ulimit memlock=-1:-1 -v $(pwd):/app -w /app oven/bun bun run test.js

test.js

import SC from 'swagger-client';

console.dir(SC);

Result was:

[Function: Swagger]

Context: swagger-client depends on openapi-path-templating. swagger-client is then used in swagger-ui-react.

char0n commented 4 months ago

Sorry I cannot help more.

kivi commented 4 months ago

@char0n cheers! you did more than enough. Thanks. I will also dig deeper soon and will tell you about it.

kivi commented 4 months ago

Maybe it's this error: https://github.com/oven-sh/bun/issues/10001

char0n commented 4 months ago

@kivi thanks for the update

rajatsandeepsen commented 4 months ago

When running swagger-ui-react on nextjs 14

image

And this is your code https://github.com/char0n/openapi-path-templating/blob/main/src/parse/callbacks/fragment-marker.js

image

char0n commented 4 months ago

@rajatsandeepsen here is POC running on Node.js >=16 and building latest swagger-ui-react https://github.com/char0n/swagger-ui-nextjs. I see no issues.

rajatsandeepsen commented 4 months ago

Okay.. but why can't you use "apg-lite" as normal import?

I still don't understand the use of "#"

It'll make easier for all of us 😊

char0n commented 4 months ago

@rajatsandeepsen because "apg-lite" is pure-esm and it would not work with swagger-client, which must work isomorphic-ally (running in Node.js >= 12.20.0 (ESM + CommonJS) and Browser).

If swagger-client (which swagger-ui-react uses) is used in CommonJS environment, it would just crash if we wouldn't be doing override with imports field.

I'll try to think of some compatible alternative, but all the modern tooling and very old Node.js supports imports field and it should just be an implementation detail for you (you shouldn't even be aware that it's there).

noahhai commented 3 months ago

For anyone else dealing with this - I have a very similar setup as the example repo, but using typescript. I did not add the polyfill node --require ./ss-polyfills/File.js - still getting an issue.

I fixed by running:

pnpm patch  openapi-server-url-templating
pnpm patch openapi-path-templating

then replacing "#apg-lite" with "apg-lite" everywhere except in package.json for both packages, and committing the patch.

Example change:

-var _apgLite = require("#apg-lite");
+var _apgLite = require("apg-lite");
char0n commented 3 months ago

I'll release a new minor version without the imports field and I'll do the patching during the release process.

char0n commented 3 months ago

Addressed in https://github.com/swaggerexpert/openapi-path-templating/pull/94

char0n commented 3 months ago

Released as https://github.com/swaggerexpert/openapi-path-templating/releases/tag/v1.6.0

char0n commented 3 months ago

Whenever swagger toolchain is now re-installed, the new version of openapi-path-templating will install and auto-resolve the issue.

char0n commented 3 months ago

openapi-server-url-templating addressed in https://github.com/swaggerexpert/openapi-server-url-templating/releases/tag/v1.1.0

noahhai commented 3 months ago

Woah you are fast @char0n! Thank you so much :)