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

Contentstack package depends on isomorphic-fetch, doesn't use it #99

Open
cefn opened this issue Jul 6, 2023 · 6 comments
Open

Contentstack package depends on isomorphic-fetch, doesn't use it #99

cefn opened this issue Jul 6, 2023 · 6 comments

Comments

@cefn
Copy link

cefn commented Jul 6, 2023

In our test suite (running in node) it seems depending on contentstack creates a broken package dependency situation.

At

import fetch from 'node-fetch';
the npm contentstack package imports node-fetch. However, it doesn't declare node-fetch among its dependencies.

The "contentstack" package correctly declares isomorphic-fetch among its dependencies. The isomorphic-fetch project allows packages to be defined which are neither node- or browser- specific, with the isomorphic-fetch package resolving to the appropriate packages at runtime.

However, because contentstack incorrectly imports node-fetch instead of isomorphic-fetch this forces projects to explicitly install node-fetch to prevent runtime errors when testing in node. This defeats the purpose of isomorphism, meaning that packages which are ONLY intended for client-side deployment end up needing a node-fetch dependency which makes no sense.

@cefn
Copy link
Author

cefn commented Jul 6, 2023

For reference this was detectable (and problematic) since node-fetch was resolved (again implicitly) to a version which was installed by another package in our monorepo. Unlike the node-fetch v2 dependency declared by isomorphic-fetch, the node-fetch v3 dependency which was then resolved (implicitly because it was undeclared by contentstack) turned out to be an ESM package, leading to the following error...

 RUN  v0.32.2 /Users/superman/workspace/scratch/contentstack-schema-iac/packages/stack-renderer

 ❯ test/transform.test.ts (0)

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Failed Suites 1 ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯

 FAIL  test/transform.test.ts [ test/transform.test.ts ]
Error: require() of ES Module /Users/superman/workspace/scratch/contentstack-schema-iac/node_modules/node-fetch/src/index.js from /Users/superman/workspace/scratch/contentstack-schema-iac/node_modules/contentstack/dist/node/contentstack.js not supported.
Instead change the require of index.js in /Users/superman/workspace/scratch/contentstack-schema-iac/node_modules/contentstack/dist/node/contentstack.js to a dynamic import() which is available in all CommonJS modules.
 ❯ 809 ../../node_modules/contentstack/dist/node/contentstack.js:1:37275
 ❯ r ../../node_modules/contentstack/dist/node/contentstack.js:1:37397
 ❯ 289 ../../node_modules/contentstack/dist/node/contentstack.js:1:36890
 ❯ r ../../node_modules/contentstack/dist/node/contentstack.js:1:37397
 ❯ 777 ../../node_modules/contentstack/dist/node/contentstack.js:1:4449
 ❯ r ../../node_modules/contentstack/dist/node/contentstack.js:1:37397
 ❯ 540 ../../node_modules/contentstack/dist/node/contentstack.js:1:11074
 ❯ r ../../node_modules/contentstack/dist/node/contentstack.js:1:37397
 ❯ 533 ../../node_modules/contentstack/dist/node/contentstack.js:1:29017
 ❯ r ../../node_modules/contentstack/dist/node/contentstack.js:1:37397
 ❯ 32 ../../node_modules/contentstack/dist/node/contentstack.js:1:2627
 ❯ r ../../node_modules/contentstack/dist/node/contentstack.js:1:37397
 ❯ ../../node_modules/contentstack/dist/node/contentstack.js:1:37423
 ❯ Object.<anonymous> ../../node_modules/contentstack/dist/node/contentstack.js:1:37446
 ❯ async /Users/superman/workspace/scratch/contentstack-schema-iac/packages/stack-iac/src/deliver.ts:3:31
 ❯ async /Users/superman/workspace/scratch/contentstack-schema-iac/packages/stack-iac/src/index.ts:3:31
 ❯ async /Users/superman/workspace/scratch/contentstack-schema-iac/packages/stack-renderer/src/transform.ts:2:31
 ❯ async /Users/superman/workspace/scratch/contentstack-schema-iac/packages/stack-renderer/test/transform.test.ts:2:31

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: {
  "code": "ERR_REQUIRE_ESM",
}
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯

 Test Files  1 failed (1)
      Tests  no tests
   Start at  14:36:06
   Duration  323ms (transform 92ms, setup 0ms, collect 0ms, tests 0ms, environment 0ms, prepare 77ms)

@cefn
Copy link
Author

cefn commented Jul 6, 2023

For reference a proven version of node-fetch that can be installed by a host project to make contentstack happy is "node-fetch": "^2.6.12". However, this is an incorrect resolution for client-side packages which intend to use contentstack isomorphically (and therefore using only browser fetch in production).

@ishaileshmishra
Copy link
Member

Thank you for bringing this to our attention @cefn. We greatly appreciate your effort in reporting the issue. Our team is currently investigating the matter and looking into the details provided.

@Mcbeer
Copy link

Mcbeer commented Nov 9, 2023

Any updates on this issue?

@cefn
Copy link
Author

cefn commented May 31, 2024

Please make this package stable.

@cs-raj
Copy link
Contributor

cs-raj commented Sep 11, 2024

HI @cefn we will require some more time to check this and we will update you as soon as possible
cc: @netrajpatel

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

No branches or pull requests

4 participants