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

Fix esm exports #1719

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix esm exports #1719

wants to merge 1 commit into from

Conversation

ken-browning
Copy link

Fixes #509

Features and Changes

  1. Files in dist/esm have had their extensions changed from .js to .mjs. This is a requirement to load the files as ESM when the package.json does not include "type": "module".
  2. The package.json references to files in dist/esm have been updated accordingly.
  3. The relative imports inside of dist/esm have been updated to include a file extension.

Dependencies

Testing

I tested with this locally

diff --git a/packages/sdk-js-esm-consumer/index.cjs b/packages/sdk-js-esm-consumer/index.cjs
new file mode 100644
index 00000000..dd74bc2b
--- /dev/null
+++ b/packages/sdk-js-esm-consumer/index.cjs
@@ -0,0 +1,3 @@
+const assert = require("node:assert");
+const { GrowthBook } = require("@growthbook/growthbook");
+assert(typeof GrowthBook === "function");
diff --git a/packages/sdk-js-esm-consumer/index.mjs b/packages/sdk-js-esm-consumer/index.mjs
new file mode 100644
index 00000000..27474bba
--- /dev/null
+++ b/packages/sdk-js-esm-consumer/index.mjs
@@ -0,0 +1,3 @@
+import assert from "node:assert";
+import { GrowthBook } from "@growthbook/growthbook";
+assert(typeof GrowthBook === "function");
diff --git a/packages/sdk-js-esm-consumer/package.json b/packages/sdk-js-esm-consumer/package.json
new file mode 100644
index 00000000..a42460e4
--- /dev/null
+++ b/packages/sdk-js-esm-consumer/package.json
@@ -0,0 +1,8 @@
+{
+  "name": "sdk-js-esm-consumer",
+  "version": "1.0.0",
+  "type": "module",
+  "dependencies": {
+    "@growthbook/growthbook": "^0.29.0"
+  }
+}

Screenshots

@bttf
Copy link
Collaborator

bttf commented Nov 3, 2023

Hi @ken-browning , this looks mostly good. You have a couple merge conflicts - they should both accept changes from upstream. Also, it seems that your yarn.lock changes include several modules that aren't necessary

@ken-browning
Copy link
Author

@bttf I have rebased

@august-growthbook
Copy link
Contributor

Hi @bttf could you kindly take another look at this PR? This fixes a blocker for one of our paying customers. Thank you!

@bttf
Copy link
Collaborator

bttf commented Nov 14, 2023

Hi @ken-browning Thanks, we need some time to make sure we're not breaking anything here

@benmccann
Copy link

Hi, SvelteKit maintainer and Vite contributor here. I have a lot of experience with packages encountering these types of errors and would like to weigh in to say that this is the correct fix and I don't see anything here that would break under any of the various bundlers or runtimes.

There's a great tool called publint, which you can use to check if you've packaged your library correctly: https://publint.dev/@growthbook/growthbook@1.0.0

@bttf bttf mentioned this pull request May 3, 2024
@bttf bttf requested a review from jdorn May 3, 2024 20:41
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.

sdk-js/package.json: "type": "module"
4 participants