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

Error with new exports field on package.json #1019

Closed
uilton-oliveira opened this issue Nov 21, 2022 · 9 comments · Fixed by #1020
Closed

Error with new exports field on package.json #1019

uilton-oliveira opened this issue Nov 21, 2022 · 9 comments · Fixed by #1020
Assignees
Labels
bug Things that aren't working right in the library.

Comments

@uilton-oliveira
Copy link
Contributor

Describe the bug

i recently created a fork of shoelace and while testing it out, we are seeing a error that does not happens in production release but happens on new versions built... Looks like it is related to this PR

If i remove the new exports block, it work fine... i'm missing anything?

ERROR in ./src/externals/vendors.js 83:0-66
Module not found: Error: Package path ./dist/themes/light.css is not exported from package C:\Programacao\platform-cortex\modules\competitiva\frontend\node_modules\@cortex-intelligence\design-system (see exports field in C:\Programacao\platform-cortex\modules\competitiva\frontend\node_modules\@cortex-intelligence\design-system\package.json)

ERROR in ./src/externals/vendors.js 84:0-77
Module not found: Error: Package path ./dist/components/button/button.js is not exported from package C:\Programacao\platform-cortex\modules\competitiva\frontend\node_modules\@cortex-intelligence\design-system (see exports field in C:\Programacao\platform-cortex\modules\competitiva\frontend\node_modules\@cortex-intelligence\design-system\package.json)

ERROR in ./src/externals/vendors.js 85:0-73
Module not found: Error: Package path ./dist/components/icon/icon.js is not exported from package C:\Programacao\platform-cortex\modules\competitiva\frontend\node_modules\@cortex-intelligence\design-system (see exports field in C:\Programacao\platform-cortex\modules\competitiva\frontend\node_modules\@cortex-intelligence\design-system\package.json)

ERROR in ./src/externals/vendors.js 87:0-93
Module not found: Error: Package path ./dist/utilities/base-path.js is not exported from package C:\Programacao\platform-cortex\modules\competitiva\frontend\node_modules\@cortex-intelligence\design-system (see exports field in C:\Programacao\platform-cortex\modules\competitiva\frontend\node_modules\@cortex-intelligence\design-system\package.json)

To Reproduce

I trying to import a fresh fork of this project to a existing project, like this:

import '@cortex-intelligence/design-system/dist/themes/light.css';
import '@cortex-intelligence/design-system/dist/components/button/button.js';
import '@cortex-intelligence/design-system/dist/components/icon/icon.js';

import { setBasePath } from '@cortex-intelligence/design-system/dist/utilities/base-path.js';
// Set the base path to the folder you copied Shoelace's assets to
setBasePath('/client/js/design-system');

Demo

Tell me if its required, if so i create it.

Screenshots

If applicable, add screenshots to help explain the bug.

Browser / OS

  • OS: Windows 11
  • Browser: Chrome
  • Browser version: Latest

Additional information

@uilton-oliveira uilton-oliveira added the bug Things that aren't working right in the library. label Nov 21, 2022
@samuelstroschein
Copy link
Contributor

I quote myself here:

I refrained from updating the PR to remove main, module, types to not possibly break some things. I don't expect anything to break but propose that you remove those keys and see if everything works.
#1007 (comment)

Of course, something had to break! :D

The problem seems to stem from the fact that "exports" is a 1:1 map. Everything that is not defined in "exports" is not exported. The solution seems to be here nodejs/node#39635. The components that you are directly importing need to be explicitly exported (at least for ESM):

{
  "exports": {
    "./dist/themes/*": "./dist/themes/*.css",
    "./dist/components/*/*": "./dist/components/*/*.js"
  }
}

Can you try adding those exports?

@tao-cumplido
Copy link
Contributor

tao-cumplido commented Nov 22, 2022

Also hit this yesterday, was about to file myself.
Adding the exports as mentioned by @samuelstroschein should fix this, although only a single * is necessary, see https://nodejs.org/docs/latest-v18.x/api/packages.html#subpath-patterns

But I was wondering if this might be an opportunity to remove the dist part of the import, e.g.

{
  "exports": {
    "./themes/*.css": "./dist/themes/*.css",
    "./components/*.js": "./dist/components/*.js"
  }
}

Imports would now look like this

import '@shoelace-style/shoelace/themes/light.css';
import '@shoelace-style/shoelace/components/button/button.js';

@uilton-oliveira
Copy link
Contributor Author

uilton-oliveira commented Nov 22, 2022

@tao-cumplido @samuelstroschein I tested some variations:

"./dist/components/*/*.js": "./dist/components/*/*.js",
ERROR in ./src/externals/vendors.js 85:0-73
Module not found: Error: Package path ./dist/components/icon/icon.js is not exported from package C:\Programacao\platform-cortex\modules\competitiva\frontend\node_modules\@cortex-intelligence\design-system (see exports field in C:\Programacao\platform-cortex\modules\competitiva\frontend\node_modules\@cortex-intelligence\design-system\package.json)
"./dist/components/*.js": "./dist/components/*.js",
ERROR in ./src/externals/vendors.js 85:0-73
Module not found: Error: Package path ./dist/components/icon/icon.js is not exported from package C:\Programacao\platform-cortex\modules\competitiva\frontend\node_modules\@cortex-intelligence\design-system (see exports field in C:\Programacao\platform-cortex\modules\competitiva\frontend\node_modules\@cortex-intelligence\design-system\package.json)

The only way that seens to work is (or without dist folder, but would require all the users to change their imports, is that wanted?):

"exports": {
    ".": {
      "types": "./dist/shoelace.d.ts",
      "import": "./dist/shoelace.js"
    },
    "./dist/themes/*": "./dist/themes/*",
    "./dist/components/*": "./dist/components/*",
    "./dist/utilities/*": "./dist/utilities/*",
    "./dist/react/*": "./dist/react/*",
    "./dist/translations/*": "./dist/translations/*"
  },

I included the utilities, that is needed to setBasePath, but there's also others folders in the module, like react, translations, styles, internal ... is any of these needed? react and translations looks like yes...

@claviska
Copy link
Member

I'd love to remove dist from imports and I'm willing to make that change as long as it works for Node 14+. I haven't done much with exports like this. Are there any major concerns or drawbacks to this approach (aside from manually keeping the right paths in exports)?

I included the utilities, that is needed to setBasePath, but there's also others folders in the module, like react, translations, styles, internal ... is any of these needed? react and translations looks like yes...

The entry points listed here are the ones we want to make sure to cover.

entryPoints: [
// The whole shebang
'./src/shoelace.ts',
// Components
...(await globby('./src/components/**/!(*.(style|test)).ts')),
// Translations
...(await globby('./src/translations/**/*.ts')),
// Public utilities
...(await globby('./src/utilities/**/!(*.(style|test)).ts')),
// Theme stylesheets
...(await globby('./src/themes/**/!(*.test).ts')),
// React wrappers
...(await globby('./src/react/**/*.ts'))
],

@uilton-oliveira
Copy link
Contributor Author

uilton-oliveira commented Nov 22, 2022

@claviska what about this?

"exports": {
    ".": {
      "types": "./dist/shoelace.d.ts",
      "import": "./dist/shoelace.js"
    },
    "./dist/shoelace.js": "./dist/shoelace.js",
    "./dist/themes/*": "./dist/themes/*",
    "./dist/components/*": "./dist/components/*",
    "./dist/utilities/*": "./dist/utilities/*",
    "./dist/react/*": "./dist/react/*",
    "./dist/translations/*": "./dist/translations/*",
    "./shoelace.js": "./dist/shoelace.js",
    "./themes/*": "./dist/themes/*",
    "./components/*": "./dist/components/*",
    "./utilities/*": "./dist/utilities/*",
    "./react/*": "./dist/react/*",
    "./translations/*": "./dist/translations/*"
  },

This way it will work with or without dist folder, making it backward compatible..

@claviska
Copy link
Member

I’d rather just remove it and break it before 2.0 stable. Less confusing for users long term.

@uilton-oliveira
Copy link
Contributor Author

Ok, but it would require update it in the docs aswell...

@claviska
Copy link
Member

Yep, I'm happy to take that on. Do you want to PR the exports change and I can tackle the docs after?

@uilton-oliveira
Copy link
Contributor Author

Sure.

claviska pushed a commit that referenced this issue Nov 22, 2022
* Fix mapping in exports

Fixes #1019
- Add correct mapping for public entrypoints.

* Remove shoelace.js from from exports list

Should still works fine while importing this way: import { thing } from '@shoelace-style/shoelace';
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Things that aren't working right in the library.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants