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

Importing Keycloak breaks typescript in esModule #24923

Closed
1 task done
AnielloFalcone opened this issue Nov 22, 2023 · 10 comments · Fixed by #24974
Closed
1 task done

Importing Keycloak breaks typescript in esModule #24923

AnielloFalcone opened this issue Nov 22, 2023 · 10 comments · Fixed by #24974
Assignees
Labels
Milestone

Comments

@AnielloFalcone
Copy link

AnielloFalcone commented Nov 22, 2023

Before reporting an issue

  • I have read and understood the above terms for submitting issues, and I understand that my issue may be closed without action if I do not follow them.

Area

adapter/javascript

Describe the bug

I have a monorepo, using Yarn 4 PnP and TS 5.2.2, migrating to ESM only.

I have already the Keycloak setup working like this:

import {default as Keycloak} from 'keycloak-js';

const keycloak = new Keycloak({
    url: 'url,
    realm: 'realm',
    clientId: 'clientId'
});

when I change the type in the package.json from commonjs to module typescript complains about the usage of Keycloak with the following error:

TS2351: This expression is not constructable.
Type
typeof import("[hidden_path]/.yarn/cache/keycloak-js-npm-22.0.5-3b6331b049-3d67303944.zip/node_modules/keycloak-js/dist/keycloak")
has no construct signatures.

I tried changing the import to import * as Keycloak from 'keycloak-js';, import Keycloak from 'keycloak-js'; or even casting the type but it still gives me this error.

Version

22.0.5

Expected behavior

I expected to work as when commonjs is set as type

Actual behavior

Typescript complains about the imported Keycloak usage

How to Reproduce?

Use this repro

Anything else?

No response

@AnielloFalcone AnielloFalcone added kind/bug Categorizes a PR related to a bug status/triage labels Nov 22, 2023
@AnielloFalcone
Copy link
Author

AnielloFalcone commented Nov 22, 2023

### UPDATE

it works without any problems if I do:

import Keycloak from 'keycloak-js';

const keycloak = new Keycloak.default({
    url: 'url,
    realm: 'realm',
    clientId: 'clientId'
});

Thanks to @ernestostifano

@jonkoops
Copy link
Contributor

You should not be importing Keycloak JS from a sub-path, that will likely break in the future. You should only import it from 'keycloak-js' directly.

Please also provide a minimal reproducible example with actual code, as I am unable to reproduce your issue. An example on StackBlitz or a GitHub repo with minimal code will suffice.

@AnielloFalcone
Copy link
Author

You should not be importing Keycloak JS from a sub-path, that will likely break in the future. You should only import it from 'keycloak-js' directly.

Please also provide a minimal reproducible example with actual code, as I am unable to reproduce your issue. An example on StackBlitz or a GitHub repo with minimal code will suffice.

Yeah my bad, I should have update the import, it was one of my tests to find a solution.

Unfortunately, even if typescript does not complain anymore I still get:

Uncaught TypeError: keycloak_js__WEBPACK_IMPORTED_MODULE_1__.default.default is not a constructor
    at ./src/auth.ts (auth.ts:4:1)
    at options.factory (react refresh:6:1)
    at __webpack_require__ (bootstrap:24:1)
    at fn (hot module replacement:62:1)
    at ./src/app.tsx (bundle.js:20:66)
    at options.factory (react refresh:6:1)
    at __webpack_require__ (bootstrap:24:1)
    at fn (hot module replacement:62:1)
    at ./src/index.tsx (auth.ts:49:1)
    at options.factory (react refresh:6:1)

when I run the application

@jonkoops
Copy link
Contributor

Again, please provide a minimal and reproducible example. We cannot fix bugs we cannot reproduce.

@AnielloFalcone
Copy link
Author

AnielloFalcone commented Nov 22, 2023

@jonkoops https://github.com/AnielloFalcone/keycloak-repro

image

@AnielloFalcone
Copy link
Author

UPDATE:

@jonkoops I created a patch of your package following this comment:

Doubling the declaration files (just duplicated the file and renamed it to keycloak.d.mts) and using that import syntax fixes the issue.

"./*": {
    "import": {
        "types": "./dist/*.d.mts",    
        "default": "./dist/*.mjs"
    },
    "require": {
        "types": "./dist/*.d.ts",
        "default": "./dist/*.js"
    }
}

@jonkoops
Copy link
Contributor

Thanks for the reproduction, this helps a lot! Looking at your proposed solution, I agree that is likely the best option to add an "exports" field. In fact I think I'll make the jump and convert everything to ESM, as Keycloak JS is intended as a client-side package anyways, so CommonJS support is no longer required.

@jonkoops
Copy link
Contributor

Upon closer inspection, we still need the UMD build to statically serve Keycloak JS. This should be deprecated and removed, but now is not the time to do so. I'll create a fix that maintains the compatibility.

@jonkoops jonkoops self-assigned this Nov 23, 2023
jonkoops added a commit to jonkoops/keycloak that referenced this issue Nov 23, 2023
Closes keycloak#24923

Signed-off-by: Jon Koops <jonkoops@gmail.com>
jonkoops added a commit to jonkoops/keycloak that referenced this issue Nov 23, 2023
Closes keycloak#24923

Signed-off-by: Jon Koops <jonkoops@gmail.com>
jonkoops added a commit to jonkoops/keycloak that referenced this issue Nov 23, 2023
Closes keycloak#24923

Signed-off-by: Jon Koops <jonkoops@gmail.com>
jonkoops added a commit to jonkoops/keycloak that referenced this issue Nov 23, 2023
Closes keycloak#24923

Signed-off-by: Jon Koops <jonkoops@gmail.com>
jonkoops added a commit to jonkoops/keycloak that referenced this issue Nov 23, 2023
Closes keycloak#24923

Signed-off-by: Jon Koops <jonkoops@gmail.com>
jonkoops added a commit to jonkoops/keycloak that referenced this issue Nov 23, 2023
Closes keycloak#24923

Signed-off-by: Jon Koops <jonkoops@gmail.com>
@jonkoops jonkoops added this to the 24.0.0 milestone Nov 23, 2023
jonkoops added a commit to jonkoops/keycloak that referenced this issue Nov 23, 2023
Closes keycloak#24923

Signed-off-by: Jon Koops <jonkoops@gmail.com>
jonkoops added a commit that referenced this issue Nov 24, 2023
Closes #24923

Signed-off-by: Jon Koops <jonkoops@gmail.com>
@AnielloFalcone
Copy link
Author

Hi @jonkoops thanks for the quick fix! It will be available in the 24.0.0 or is already available in the 23.0.0 just released?

@jonkoops
Copy link
Contributor

Since it is a breaking change it will be made available for v24, unfortunately it was too late to land this in v23.

srose pushed a commit to srose/keycloak that referenced this issue Dec 20, 2023
Closes keycloak#24923

Signed-off-by: Jon Koops <jonkoops@gmail.com>
kamontat pushed a commit to kamontat/keycloak that referenced this issue Jan 20, 2024
Closes keycloak#24923

Signed-off-by: Jon Koops <jonkoops@gmail.com>
Signed-off-by: Kamontat Chantrachirathumrong <14089557+kamontat@users.noreply.github.com>
ShefeeqPM pushed a commit to ShefeeqPM/keycloak that referenced this issue Jan 27, 2024
Closes keycloak#24923

Signed-off-by: Jon Koops <jonkoops@gmail.com>
Signed-off-by: ShefeeqPM <86718986+ShefeeqPM@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants