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

Avoid using default exports #175

Merged
merged 8 commits into from
Aug 1, 2023
Merged

Avoid using default exports #175

merged 8 commits into from
Aug 1, 2023

Conversation

frederikprijck
Copy link
Member

@frederikprijck frederikprijck commented Jul 31, 2023

Description

Drops the default exports, both from the internal and the external API.
This should simplify support for CJS and ESM, see #174 .

Checklist

  • I have added documentation for new/changed functionality in this PR or in auth0.com/docs
  • All active GitHub checks for tests, formatting, and security are passing
  • The correct base branch is being used, if not the default branch

@frederikprijck frederikprijck requested a review from a team as a code owner July 31, 2023 17:26
rollup.config.ts Outdated
@@ -17,7 +17,7 @@ const plugins = [
];

export default defineConfig([{
input: "lib/index.standalone.ts",
input: "lib/index.ts",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the input is now the same for all files, we can put it in a variable so it can be re-used.

Copy link
Contributor

@jonkoops jonkoops left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jonkoops
Copy link
Contributor

Actually have some things that can go into this PR, let me create some changes.

@jonkoops
Copy link
Contributor

See #176

@jonkoops
Copy link
Contributor

I am going to take a look and see if we can somehow keep the UMD bundle compatible. I agree that we should avoid breaking this if possible.

@jonkoops
Copy link
Contributor

@frederikprijck created #181 to handle this, I think for the sake of backwards compatibility keeping a separate index for UMD is alright. The named exports API also seems really quirky for this one.

Copy link
Contributor

@jonkoops jonkoops left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jonkoops
Copy link
Contributor

jonkoops commented Aug 1, 2023

@frederikprijck @adamjmcgrath shall we merge this in so work can continue on #174?

@frederikprijck frederikprijck enabled auto-merge (squash) August 1, 2023 13:19
Copy link
Contributor

@ewanharris ewanharris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, pushed a commit to fix the file reference in static/index.html

@frederikprijck frederikprijck merged commit 29ccbc2 into beta Aug 1, 2023
1 check passed
@frederikprijck frederikprijck deleted the fix/default-exports branch August 1, 2023 15:30
frederikprijck added a commit that referenced this pull request Oct 27, 2023
Co-authored-by: Jon Koops <jonkoops@gmail.com>
Co-authored-by: Ewan Harris <ewan.harris@okta.com>
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.

3 participants