feat: supporting esm import #824

anuragmerndev · 2024-04-29T20:57:52Z

add the exports in package.json to support esm import for js and ts

Summary of change

This pull request addresses issue #783 by adding subpath patterns exports to the package.json file. These exports support ESM (ECMAScript Modules) imports for both JavaScript and TypeScript files, improving compatibility and ease of use for developers. Now, imports like import Session from "supertokens-node/recipe/session"; can be used without specifying the index.js file explicitly.

Related issues

Test Plan

Update the package.json file with the provided exports configuration.
Ensure the project builds without errors.
Verify that existing imports using ESM syntax work as expected.
Test importing modules in both JavaScript and TypeScript files.
Run unit tests to confirm that the changes do not introduce regressions.
Check for any compatibility issues with existing codebases using the library.

supertokens-issue-783-support-esm.mp4

Documentation changes

No documentation changes are required for this PR.

Checklist for important updates

Remaining TODOs for this PR

No additional tasks are required for this PR.

ellipsis-dev · 2024-04-29T20:57:56Z

Your free trial has expired. To continue using Ellipsis, sign up at https://app.ellipsis.dev for $20/developer/month. If you have any questions, reach us at [email protected]

netlify · 2024-04-29T20:58:09Z

✅ Deploy Preview for astounding-pegasus-21c111 canceled.

Name	Link
🔨 Latest commit	`f3e92fb`
🔍 Latest deploy log	https://app.netlify.com/sites/astounding-pegasus-21c111/deploys/66300a4391332d0008152fac

netlify · 2024-04-29T20:58:32Z

✅ Deploy Preview for precious-marshmallow-968a81 canceled.

Name	Link
🔨 Latest commit	`f3e92fb`
🔍 Latest deploy log	https://app.netlify.com/sites/precious-marshmallow-968a81/deploys/66300a4373b9540008771d2e

package.json

ITenthusiasm · 2024-05-01T13:02:16Z

From the video, I can see that the build works; but I don't see the examples of ESM-based/CJS-based/TypeScript Node applications working properly. How did those go?

ITenthusiasm · 2024-05-01T13:04:33Z

Also just giving a reminder to target the proper version branch, as mentioned in the PR checklist.

anuragmerndev · 2024-05-01T19:34:40Z

From the video, I can see that the build works; but I don't see the examples of ESM-based/CJS-based/TypeScript Node applications working properly. How did those go?

I have updated the video last video didn't capture the other two windows
attaching here as well

supertokens-issue-783-support-esm.mp4

anuragmerndev · 2024-05-01T19:35:11Z

Also just giving a reminder to target the proper version branch, as mentioned in the PR checklist.

Okay I have updated to the latest unreleased branch

ITenthusiasm · 2024-05-02T15:35:09Z

Thanks for the update! As a heads up, I probably won't be able to look at this until the weekend. There's also rishabhpoddar when he has more availability, though. I'm mainly here to help with making sure that the import/export configuration looks normal.

An optional recommendation: Your commit history might be clearer if you rebase your current changes onto the latest version of the target branch instead of merging that branch's features into your own.

ITenthusiasm · 2024-05-02T15:39:11Z

So far the new video looks promising though!

anuragmerndev · 2024-05-02T16:06:53Z

Thanks for the update! As a heads up, I probably won't be able to look at this until the weekend. There's also rishabhpoddar when he has more availability, though. I'm mainly here to help with making sure that the import/export configuration looks normal.

An optional recommendation: Your commit history might be clearer if you rebase your current changes onto the latest version of the target branch instead of merging that branch's features into your own.

Sure I'll do the rebase

ITenthusiasm · 2024-05-06T12:12:06Z

Hey friend, sorry I wasn't able to get to this. The weekend actually turned out much busier than I expected. But it looks like things should calm down after today so hopefully I'll be able to review soon.

(I know the code change is small. But I want to be able to test the changes as well to double-verify that no unsuspecting breaking changes would sneak in. I'm still a little surprised that we wouldn't need separate files for handling CJS/ESM exports.)

anuragmerndev · 2024-05-07T06:25:16Z

Sure you can test the same and do let me know if you have any questions on the same

Talking about the require and import in the nodejs documentation for conditional exports it's provided that we can separate the cjs and mjs if we are having two files but we can also give default key which will work for both the scenarios here's the link to documentation

ITenthusiasm · 2024-05-11T14:44:54Z

@anuragmerndev Can you link this to #459 as well to help with cleaning up the issues?

Separately, I'm probably going to try to pull this branch locally to test (just to doubly make sure). My first suspicion as to why this would work is that ESM is able to import from CJS (but not the other way around). If that's the case then yeah, this should be smooth sailing.

ITenthusiasm

LGTM (Sorry this took so long to review. Been really busy.)

@rishabhpoddar

I only tested importing from supertokens-node/recipe/emailpassword. (@anuragmerndev's example uses significantly more imports.) But for that import, I can confirm that requireing worked just fine, and importing worked for type=module node apps. (importing does not work if the semantic exports property is excluded from the package.json file in supertokens-node).

Some Brief Background

CJS applications cannot import from ESM applications. But ESM applications can import from both CJS and ESM applications. The supertokens-node package does not identify as an ESM package (because type="module" is not in the package.json file); so it is a CJS package. Therefore, when an ESM application tries to import from supertokens-node, it sees it as a CJS package, and the import works fine (because ESM can handle those). Of course, regular CJS applications have no problem importing from supertokens-node currently.

Additional Notes

If there are any other paths that people should be allowed to import/require from, they need to be added to exports in package.json. Otherwise, consumers won't be allowed to access those paths. The exports property in package.json is effectively a "whitelist". If it's sufficient to only allow imports from supertokens-node/recipe/**/*, then I guess this PR is good to go. But if other paths need to be exposed, it would be helpful to know those.

Not sure what you wanted to do about tests. There's probably a way to write those? The test would basically involve successfully importing/requireing without errors -- if you wanted to automate that.

If you guys added documentation on how to write imports for applications using ESM, then that documentation will need to be re-updated when this gets merged. (Otherwise, don't worry about it.)

@anuragmerndev

Thanks for looking into this. (And sorry again for the delay.) I'm sure this change will make a lot of people happy. Could you share a code snippet of the test file that you used in the video? That way, if anyone else wants to test locally, they'll have the ability to do so with ease.

anuragmerndev · 2024-05-12T06:51:33Z

LGTM (Sorry this took so long to review. Been really busy.)

@rishabhpoddar

I only tested importing from supertokens-node/recipe/emailpassword. (@anuragmerndev's example uses significantly more imports.) But for that import, I can confirm that requireing worked just fine, and importing worked for type=module node apps. (importing does not work if the semantic exports property is excluded from the package.json file in supertokens-node).

Some Brief Background

CJS applications cannot import from ESM applications. But ESM applications can import from both CJS and ESM applications. The supertokens-node package does not identify as an ESM package (because type="module" is not in the package.json file); so it is a CJS package. Therefore, when an ESM application tries to import from supertokens-node, it sees it as a CJS package, and the import works fine (because ESM can handle those). Of course, regular CJS applications have no problem importing from supertokens-node currently.

Additional Notes

If there are any other paths that people should be allowed to import/require from, they need to be added to exports in package.json. Otherwise, consumers won't be allowed to access those paths. The exports property in package.json is effectively a "whitelist". If it's sufficient to only allow imports from supertokens-node/recipe/**/*, then I guess this PR is good to go. But if other paths need to be exposed, it would be helpful to know those.

Not sure what you wanted to do about tests. There's probably a way to write those? The test would basically involve successfully importing/requireing without errors -- if you wanted to automate that.

If you guys added documentation on how to write imports for applications using ESM, then that documentation will need to be re-updated when this gets merged. (Otherwise, don't worry about it.)

@anuragmerndev

Thanks for looking into this. (And sorry again for the delay.) I'm sure this change will make a lot of people happy. Could you share a code snippet of the test file that you used in the video? That way, if anyone else wants to test locally, they'll have the ability to do so with ease.

@ITenthusiasm Thank you for the update on the issue, I have created a repository with a README outlining the test procedure for this fix. supertokens-node-import-test

And thank you @ITenthusiasm for sharing your time on checking this PR, means a lot.

@rishabhpoddar

Regarding the exports, as @ITenthusiasm mentioned, only the files within the 'recipe' directory are currently accessible due to the whitelist configuration. If there are any additional paths that should be exposed, please let me know. During my review of the documentation for supertokens-node usage, I only encountered imports related to recipes. If there are other imports we need to include, I'll be happy to update the PR accordingly

rishabhpoddar · 2024-05-12T12:28:11Z

Thanks @anuragmerndev and @ITenthusiasm for the work so far, really appreciate it. Someone from our team will have a look at this too soon

porcellus

Hi,
First of all, sorry for the late review. We've been swamped releasing 18.0 and other projects.

As for the PR, I have some concerns:

Please update the PR (and the target branch) to the latest
Some entry points are missing (see my comment), so ideally, we'd like to figure out a way to "test" if this happens again. This is not necessarily a blocker for this PR, but any ideas are welcome.
If we can test that the defined exports match some files, that'd also be awesome.
We need to allow importing from the build output directly, as we've seen some use cases that require that. I know it's less than ideal, but we want to enable pretty extreme customizations. I was thinking of adding something like the following to the exports obj, but it seems relatively awkward, so any ideas are welcome:

        "./lib/*.js": {
            "types": "./lib/*.d.ts",
            "default": "./lib/*.js"
        },
        "./lib/*": {
            "types": "./lib/*.d.ts",
            "default": "./lib/*.js"
        }

Can we also support existing apps that import us using a ".js" module specifier? Otherwise, this PR will be a breaking change.

package.json

add the exports in package.json to support esm import for js and ts

anuragmerndev · 2024-06-02T16:25:45Z

Hi, First of all, sorry for the late review. We've been swamped releasing 18.0 and other projects.

As for the PR, I have some concerns:

Please update the PR (and the target branch) to the latest

Some entry points are missing (see my comment), so ideally, we'd like to figure out a way to "test" if this happens again. This is not necessarily a blocker for this PR, but any ideas are welcome.

If we can test that the defined exports match some files, that'd also be awesome.

We need to allow importing from the build output directly, as we've seen some use cases that require that. I know it's less than ideal, but we want to enable pretty extreme customizations. I was thinking of adding something like the following to the exports obj, but it seems relatively awkward, so any ideas are welcome:
        "./lib/*.js": {
            "types": "./lib/*.d.ts",
            "default": "./lib/*.js"
        },
        "./lib/*": {
            "types": "./lib/*.d.ts",
            "default": "./lib/*.js"
        }
Can we also support existing apps that import us using a ".js" module specifier? Otherwise, this PR will be a breaking change.

Hello @porcellus

No worries about the late reviews, In fact, thank you for checking the PR also here's the updates on your concerns

I have updated the PR target branch to 18.0.
I'll add the missing entry points in the final commit I am working on.
I can provide the repository which I shared before in the comments for testing.
For the build exports, I am researching to find a better solution; otherwise, we can proceed with the approach you provided.
We will need to add something like the following to support existing apps using the ".js" module specifier, so this PR won't be a breaking change:

  "./recipe/*": {
        "types": "./recipe/*/index.d.ts",
        "default": "./recipe/*/index.js"
    },
    // for index.js support
    "./recipe/*/index.js": {
        "types": "./recipe/*/index.d.ts",
        "default": "./recipe/*/index.js"
    }

Also I wanted to confirm that do we need to provide this .js support for all the exports you mentioned in the comments ?

porcellus · 2024-06-03T10:09:12Z

I can provide the repository which I shared before in the comments for testing.

I meant some way of doing this in an automated test, especially testing that we didn't miss anything.

For the build exports, I am researching to find a better solution; otherwise, we can proceed with the approach you provided.

That'd be awesome.

We will need to add something like the following to support existing apps using the ".js" module specifier...

Sounds good

Also I wanted to confirm that do we need to provide this .js support for all the exports you mentioned in the comments?

Yeah, all entrypoints should work either way, otherwise we might break some apps.

adding the support for .js module exports and support for types, nextjs, framework and recipe module exports

anuragmerndev · 2024-06-03T17:39:13Z

Hello @porcellus,

I have pushed the updated code with support for rest of modules and the .js modules for existing apps along with build folder.

About the test thing I am still researching how can we automate tests for this specific PR, haven't found any till now.

Kindly review the same and let me know for any updates.

porcellus · 2024-06-07T16:35:25Z

I've reviewed this and it looks good. I'll merge this into an intermediary branch, because I need to add some further changes (I've figured out a way to automatically test it) and add a changelog entry/increase the version number.

anuragmerndev · 2024-06-07T16:38:59Z

I've reviewed this and it looks good. I'll merge this into an intermediary branch, because I need to add some further changes (I've figured out a way to automatically test it) and add a changelog entry/increase the version number.

Alright thank you,

Also can you please share the logic for automated tests

porcellus · 2024-06-07T17:48:02Z

You can check out the test here. It's far from pretty, but it already showed some errors we had.

Also, if you'd like, please feel free to add yourself as a contributor in the supertokens-core repo. (similar to this PR)
Thank you for contributing to SuperTokens.

@porcellus

Added Anurag Srivastava to contributors list as mentioned by @porcellus supertokens/supertokens-node#824 (comment)

@porcellus

Added Anurag Srivastava to contributors list as mentioned by @porcellus supertokens/supertokens-node#824 (comment)

anuragmerndev · 2024-06-07T18:27:34Z

You can check out the test here. It's far from pretty, but it already showed some errors we had.

Also, if you'd like, please feel free to add yourself as a contributor in the supertokens-core repo. (similar to this PR) Thank you for contributing to SuperTokens.

Thank you for letting me contribute will do alot more of it
Also here's the PR to the Contributor update Do let me know if need any update

* feat: supporting esm import (#824) * feat: supporting esm import add the exports in package.json to support esm import for js and ts * feat: support for .js module export adding the support for .js module exports and support for types, nextjs, framework and recipe module exports * test: add a test to check our export mapping * fix: update export mapping * chore: update version number * chore: add new related checklist item to PR template * fix: remove unnecessary/broken entry point * test: add explanation comment --------- Co-authored-by: Anurag Srivastava <[email protected]>

anuragmerndev mentioned this pull request Apr 30, 2024

Feat: Support Convenient Directory Imports for Developers Using ESM #783

Open

ITenthusiasm reviewed May 1, 2024

View reviewed changes

package.json Show resolved Hide resolved

anuragmerndev changed the base branch from master to 17.1 May 1, 2024 19:33

anuragmerndev force-pushed the feat/support-directory-import-esm branch from f1bc291 to 26270d9 Compare May 3, 2024 19:10

ITenthusiasm approved these changes May 11, 2024

View reviewed changes

anuragmerndev force-pushed the feat/support-directory-import-esm branch from 26270d9 to 07fce0f Compare May 12, 2024 08:24

porcellus requested changes May 30, 2024

View reviewed changes

package.json Outdated Show resolved Hide resolved

feat: supporting esm import

ae2fe2d

add the exports in package.json to support esm import for js and ts

anuragmerndev force-pushed the feat/support-directory-import-esm branch from 07fce0f to ae2fe2d Compare June 2, 2024 12:42

anuragmerndev changed the base branch from 17.1 to 18.0 June 2, 2024 12:42

feat: support for .js module export

0a4894b

adding the support for .js module exports and support for types, nextjs, framework and recipe module exports

porcellus approved these changes Jun 7, 2024

View reviewed changes

porcellus changed the base branch from 18.0 to feat/support_esm_imports June 7, 2024 16:35

porcellus merged commit eada4be into supertokens:feat/support_esm_imports Jun 7, 2024
4 of 7 checks passed

porcellus mentioned this pull request Jun 7, 2024

feat!: better support for ESM imports #850

Merged

13 tasks

anuragmerndev added a commit to anuragmerndev/supertokens-core that referenced this pull request Jun 7, 2024

docs(README.md): Added Anurag Srivastava to contributors

6520155

Added Anurag Srivastava to contributors list as mentioned by @porcellus supertokens/supertokens-node#824 (comment)

anuragmerndev mentioned this pull request Jun 7, 2024

docs(README.md): Added Anurag Srivastava to contributors supertokens/supertokens-core#1003

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: supporting esm import #824

feat: supporting esm import #824

anuragmerndev commented Apr 29, 2024 •

edited

Loading

ellipsis-dev bot commented Apr 29, 2024

netlify bot commented Apr 29, 2024 •

edited

Loading

netlify bot commented Apr 29, 2024 •

edited

Loading

ITenthusiasm commented May 1, 2024

ITenthusiasm commented May 1, 2024

anuragmerndev commented May 1, 2024

anuragmerndev commented May 1, 2024

ITenthusiasm commented May 2, 2024

ITenthusiasm commented May 2, 2024 •

edited

Loading

anuragmerndev commented May 2, 2024

ITenthusiasm commented May 6, 2024

anuragmerndev commented May 7, 2024

ITenthusiasm commented May 11, 2024

ITenthusiasm left a comment •

edited

Loading

anuragmerndev commented May 12, 2024 •

edited

Loading

@rishabhpoddar

Some Brief Background

Additional Notes

@anuragmerndev

rishabhpoddar commented May 12, 2024

porcellus left a comment

anuragmerndev commented Jun 2, 2024 •

edited

Loading

porcellus commented Jun 3, 2024

anuragmerndev commented Jun 3, 2024 •

edited

Loading

porcellus commented Jun 7, 2024

anuragmerndev commented Jun 7, 2024

porcellus commented Jun 7, 2024

anuragmerndev commented Jun 7, 2024

feat: supporting esm import #824

feat: supporting esm import #824

Conversation

anuragmerndev commented Apr 29, 2024 • edited Loading

Summary of change

Related issues

Test Plan

Documentation changes

Checklist for important updates

Remaining TODOs for this PR

ellipsis-dev bot commented Apr 29, 2024

netlify bot commented Apr 29, 2024 • edited Loading

✅ Deploy Preview for astounding-pegasus-21c111 canceled.

netlify bot commented Apr 29, 2024 • edited Loading

✅ Deploy Preview for precious-marshmallow-968a81 canceled.

ITenthusiasm commented May 1, 2024

ITenthusiasm commented May 1, 2024

anuragmerndev commented May 1, 2024

anuragmerndev commented May 1, 2024

ITenthusiasm commented May 2, 2024

ITenthusiasm commented May 2, 2024 • edited Loading

anuragmerndev commented May 2, 2024

ITenthusiasm commented May 6, 2024

anuragmerndev commented May 7, 2024

ITenthusiasm commented May 11, 2024

ITenthusiasm left a comment • edited Loading

Choose a reason for hiding this comment

@rishabhpoddar

Some Brief Background

Additional Notes

@anuragmerndev

anuragmerndev commented May 12, 2024 • edited Loading

@rishabhpoddar

Some Brief Background

Additional Notes

@anuragmerndev

@rishabhpoddar

rishabhpoddar commented May 12, 2024

porcellus left a comment

Choose a reason for hiding this comment

anuragmerndev commented Jun 2, 2024 • edited Loading

porcellus commented Jun 3, 2024

anuragmerndev commented Jun 3, 2024 • edited Loading

porcellus commented Jun 7, 2024

anuragmerndev commented Jun 7, 2024

porcellus commented Jun 7, 2024

anuragmerndev commented Jun 7, 2024

anuragmerndev commented Apr 29, 2024 •

edited

Loading

netlify bot commented Apr 29, 2024 •

edited

Loading

netlify bot commented Apr 29, 2024 •

edited

Loading

ITenthusiasm commented May 2, 2024 •

edited

Loading

ITenthusiasm left a comment •

edited

Loading

anuragmerndev commented May 12, 2024 •

edited

Loading

anuragmerndev commented Jun 2, 2024 •

edited

Loading

anuragmerndev commented Jun 3, 2024 •

edited

Loading