Esri / calcite-design-system

A monorepo containing the packages for Esri's Calcite Design System
https://developers.arcgis.com/calcite-design-system/
Other
280 stars 76 forks source link

Crashes on Node.js due to lack of `exports` map in `package.json` #5077

Open benmccann opened 2 years ago

benmccann commented 2 years ago

Actual Behavior

Crashes when running in Node.js without a bundler

The simplest fix and best is to add an exports map in package.json. Alternatively, all packages referring to this one could be updated to do CommonJS-style imports (i.e. import * as components from '@arcgis/calcite-components')

This has been reported as an issue in multiple places:

This also affects other packages such as @esri/calcite-colors

Expected Behavior

Package loads and runs

Reproduction Sample

https://github.com/georgeboot/svelte-kit-named-export-issue

Reproduction Steps

git clone git@github.com:georgeboot/svelte-kit-named-export-issue.git
cd svelte-kit-named-export-issue
npm install
npm run dev

Visit http://localhost:5173/arcgis to see the error

Reproduction Version

1.0.0-beta.82

Relevant Info

Named export 'setAssetPath' not found. The requested module '@esri/calcite-components/dist/components/index.js' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from '@esri/calcite-components/dist/components/index.js';
const {setAssetPath: o}from"@esri/calcite-components/dist/components/index.js";import{getAssetUrl: s}from"../../assets.js";import"../../core/has.js";import{makeAbsolute: t}from"../../core/urlUtils.js";let e;function r(){o(t(s(e)))}e="components/assets";export{r: commitAssetPath} = pkg;

Regression?

No response

github-actions[bot] commented 2 years ago

It looks like this issue is missing some information:

Issues without reproduction samples may not be prioritized. If you were unable to create a sample, please try to answer any questions that arise once development begins. Thanks for your understanding.

elliott-with-the-longest-name-on-github commented 2 years ago

Just copying the repro from one of our issues to make it easier on the lib authors.

  1. Clone the following repo: https://github.com/georgeboot/svelte-kit-named-export-issue
  2. Run npm install
  3. Run npm run dev
  4. Visit http://localhost:5173/arcgis
  5. See error

I see now that Ben included the link in his original comment -- I just saw the bot message and assumed. Oops. 🙄

benmccann commented 2 years ago

A codepen, codesandbox, or jsbin that reproduces the issue.

None of those would work, because they only run the JS in the browser and this is a Node.js issue. I could provide a stackblitz that reproduces the issue, but don't really see the point since I've already provided a github repository

Could we get the incomplete issue report label removed since the bot incorrectly applied it?

benelan commented 2 years ago

It's removed, sorry about that!

am-maneaters commented 2 years ago

We're also running into this issue using Vitest in our project

benelan commented 2 years ago

The Esri Community forum that you linked has the correct answer; you will need to use dynamic imports. Here is a SvelteKit example using the ArcGIS API for JavaScript: https://github.com/benelan/arcgis-esm-samples/tree/main/jsapi-sveltekit-template

Does that help? If not can you please elaborate on the issue you are facing and/or provide another repro sample that better demonstrates the problem? Thanks!

benmccann commented 2 years ago

This issue is asking for exports to be added to the package.json so that users are no longer forced to use dynamic imports, but can use regular static imports

benelan commented 2 years ago

I'm not very familiar with SvelteKit, but wouldn't static imports cause the ArcGIS JavaScript API to render on the server? They don't support server side rendering, so even if we add exports the reproduction sample will not work.

benmccann commented 2 years ago

Yes, it would cause it to render on the server. I had assumed these components would work on the server because you distribute CommonJS files. I'm a maintainer of SvelteKit and haven't used these components myself, but wanted to get to the bottom of this as we've seen users running into it. What scenario are the CommonJS files intended for if not for running on the server? It could make sense to remove the CommonJS files and add "type": "module" to the package.json if these are only intended to run in the browser.

benelan commented 2 years ago

Sorry if I wasn't clear, I was saying that the ArcGIS API for JavaScript (@arcgis/core) cannot render on the server. So the repro app won't work even after fixing the errors from this issue, you will still need to use dynamic imports.

Putting the repro sample aside, Calcite Components are web components and they rely on browser APIs. So using static imports won't work, you will need to use the hydrate build. Here is some more documentation about why CommonJS modules are provided. I'll fix the Calcite Component specific errors but it won't fix any of the problems you are facing.

benmccann commented 2 years ago

I'm not following the explanation for why CommonJS modules are provided. That link just says you have to use rollup-plugin-commonjs if you're using Rollup and encounter a CommonJS dependency. But that's not really a reason to make dependencies be CommonJS in the first place. Rollup users would much prefer if you didn't make them utilize rollup-plugin-commonjs to handle a given component

benelan commented 2 years ago

Sorry my thoughts got shmushed together. I meant:

Here is an example using CommonJS. Here is some documentation about how I can fix the Calcite Components errors, but it won't fix the problems you are facing.

Stencil uses rollup-plugin-commonjs internally and I can use the namedExports property to fix the errors.

I'll have to do some invesitgating about whether removing CJS entrypoints will affect the hydrate build. We don't get a lot of Node users so I'm not sure at the moment.

benelan commented 1 year ago

After a long wait, Stencil got back to us with the following:

Regarding the CJS files, there may be some undocumented dependencies, so I'd suggest not removing them now. One of the items on the Stencil team's list is to investigate and document any dependencies.

I'm marking this issue as blocked until Stencil figures it out on their end and lets us know.

benelan commented 1 year ago

Still going back and forth with Stencil's support over this, but they said that Vitetest isn't officially supported. We are going to try escalating the issue, thanks for your patience everyone.

Isaac-o commented 11 months ago

Was there anymore information regarding this issue. Seem to be running into the same thing.

Isaac-o commented 11 months ago

The Esri Community forum that you linked has the correct answer; you will need to use dynamic imports. Here is a SvelteKit example using the ArcGIS API for JavaScript: https://github.com/benelan/arcgis-esm-samples/tree/main/jsapi-sveltekit-template

Does that help? If not can you please elaborate on the issue you are facing and/or provide another repro sample that better demonstrates the problem? Thanks!

I was also going to have a look at this solution but it seems to no longer be available?

I am getting the above error due to Vitest.

jonathandawe-geollect commented 10 months ago

@benelan - I'm having major issues trying to use vitetest with the ArcGIS JS API, and I think it might be the issue you are referring to above. Do you know whether this is something that is being progressed?

SyntaxError: Named export 'getAssetPath' not found. The requested module '@stencil/core/internal/client/index.js' is a CommonJS module, which may not support all module.exports as named exports.  
CommonJS modules can always be imported via the default export, for example using:

import pkg from '@stencil/core/internal/client/index.js';
const { getAssetPath, setAssetPath, setNonce, setPlatformOptions } = pkg;
kgalliher commented 10 months ago

FWIW: I can reproduce a similar error with a Vitest - Node setup. I can create a FeatureLayer but a simple attribute query blows up:

message: 'Named export \'getAssetPath\' not found. The requested module \'@stencil/core/internal/client/index.js\' is a CommonJS module, which may not support all module.exports as named exports.

Because I am using my own enterprise, I need to provide an apiKey to my feature layer arguments:

const fl: FeatureLayer = new FeatureLayer({
    url: "https://machine.domain.com/server/rest/services/BigWorm/FeatureServer/1",
    apiKey: "bBZN5clYULkqT-----blah"
});

With a correct token I can access the FeatureLayer's attributes and my tests pass. However, I get more certificate and authentication errors in debug mode using VS Code.

pineapplegum commented 8 months ago

Trying to run Vitest in a project using ArcGIS SDK for javascript causes this:

`SyntaxError: Named export 'setAssetPath' not found. The requested module '@stencil/core/internal/client/index.js' is a CommonJS module, which may not support all module.exports as named exports. CommonJS modules can always be imported via the default export, for example using:

import pkg from '@stencil/core/internal/client/index.js'; const { setAssetPath, setPlatformOptions } = pkg;`

jonathandawe-geollect commented 6 months ago

@benelan - I'm having major issues trying to use vitetest with the ArcGIS JS API, and I think it might be the issue you are referring to above. Do you know whether this is something that is being progressed?

SyntaxError: Named export 'getAssetPath' not found. The requested module '@stencil/core/internal/client/index.js' is a CommonJS module, which may not support all module.exports as named exports.  
CommonJS modules can always be imported via the default export, for example using:

import pkg from '@stencil/core/internal/client/index.js';
const { getAssetPath, setAssetPath, setNonce, setPlatformOptions } = pkg;

Just to follow this up as it might be helpful to others. I found that I had the best results when testing with the ArcGIS JS API by implementing a mocks file for the different Classes I was using.

For example, mocking the ZoomViewModel.ts in the folder: \__mocks__\@arcgis\core\widgets\Zoom\ZoomViewModel.ts

import { vi } from "vitest";

const ZoomVM = vi.fn();
ZoomVM.prototype.zoomIn = vi.fn();
ZoomVM.prototype.zoomOut = vi.fn();
ZoomVM.prototype.state = "ready";
ZoomVM.prototype.canZoomIn = true;
ZoomVM.prototype.canZoomOut = false;

export default ZoomVM;

You can then test this in a component like so (example for React):

import MapView from "@arcgis/core/views/MapView";
import ZoomVM from "@arcgis/core/widgets/Zoom/ZoomViewModel";
import { vi } from "vitest";
import { fireEvent, themedRender } from "../../../test/test-utils";
import ZoomMenu from "./ZoomMenu";

vi.mock("@arcgis/core/views/MapView");
vi.mock("@arcgis/core/widgets/Zoom/ZoomViewModel");

describe("ZoomMenu", () => {
  let mapView: __esri.MapView;
  let zoomVM: ZoomVM;

  beforeEach(() => {
    // Render the ZoomMenu component:
    mapView = new MapView();
    zoomVM = new ZoomVM();
  });
  it("renders ZoomMenu component", () => {
    const view = themedRender(<ZoomMenu />);

    // Get the zoom buttons:
    const zoomInButton = view.getByRole("button", { name: "Zoom in" });
    const zoomOutButton = view.getByRole("button", { name: "Zoom out" });

    fireEvent.click(zoomInButton);
    expect(zoomVM.zoomIn).toHaveBeenCalled();

    fireEvent.click(zoomOutButton);
    expect(zoomVM.zoomOut).not.toHaveBeenCalled();
  });
});
slegaitis commented 3 weeks ago

Having the same issue with any components that use MapView