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

Event Type exports virally expose code to build #2350

Open
Westbrook opened this issue Jan 20, 2025 · 2 comments
Open

Event Type exports virally expose code to build #2350

Westbrook opened this issue Jan 20, 2025 · 2 comments

Comments

@Westbrook
Copy link

Because ./dist/events/* is not surfaced for access in a consuming project,

shoelace/package.json

Lines 13 to 31 in 6761fdc

"exports": {
".": {
"types": "./dist/shoelace.d.ts",
"import": "./dist/shoelace.js"
},
"./dist/custom-elements.json": "./dist/custom-elements.json",
"./dist/shoelace.js": "./dist/shoelace.js",
"./dist/shoelace-autoloader.js": "./dist/shoelace-autoloader.js",
"./dist/themes": "./dist/themes",
"./dist/themes/*": "./dist/themes/*",
"./dist/components": "./dist/components",
"./dist/components/*": "./dist/components/*",
"./dist/utilities": "./dist/utilities",
"./dist/utilities/*": "./dist/utilities/*",
"./dist/react": "./dist/react/index.js",
"./dist/react/*": "./dist/react/*",
"./dist/translations": "./dist/translations",
"./dist/translations/*": "./dist/translations/*"
},

And the event types are reexported without the type label:

export * from './events/events.js';

While the project does not list sideeffects in package.json, imports like import type { SlSelectEvent } from "@shoelace-style/shoelace"; can virally expose the entirety of the project to bundlers.

The easiest fix is likely surfacing ./dist/events/* as a package export so that event types can be accessed externally via a path with no runtime code allowing the consumer to fix their own problems here.

Alternative options require a little testing, that I think will work out, but are less simple/guaranteed:

  • add the type label should fix this issue at the TypeScript level; (IIUC) it's absence confused the build into being unable to separate types from actual code in that import.
  • add sideeffect listings to package.json to prevent vitality at the bundle level. This can be difficult to maintain over time, but is a great way to ensure consumers have the smallest amount of code built into their applications.
@Westbrook
Copy link
Author

Related, the use of built and bundled files in the published versions of Shoelace (e.g. chunk.KLASTODS.js et al) make even knowing that unused code has been included in the build as visualizers will likely list it all like this:

Image

When the same app really needed less that a third of that code:

Image

The current approach requires a consumer to read into the actual files themselves and the unused lines to track down that components you're not leveraging are being included in the final build.

@KonnorRogers
Copy link
Collaborator

Because ./dist/events/* is not surfaced for access in a consuming project,

shoelace/package.json

Lines 13 to 31 in 6761fdc
"exports": {
".": {
"types": "./dist/shoelace.d.ts",
"import": "./dist/shoelace.js"
},
"./dist/custom-elements.json": "./dist/custom-elements.json",
"./dist/shoelace.js": "./dist/shoelace.js",
"./dist/shoelace-autoloader.js": "./dist/shoelace-autoloader.js",
"./dist/themes": "./dist/themes",
"./dist/themes/": "./dist/themes/",
"./dist/components": "./dist/components",
"./dist/components/": "./dist/components/",
"./dist/utilities": "./dist/utilities",
"./dist/utilities/": "./dist/utilities/",
"./dist/react": "./dist/react/index.js",
"./dist/react/": "./dist/react/",
"./dist/translations": "./dist/translations",
"./dist/translations/": "./dist/translations/"
},

And the event types are reexported without the type label:

shoelace/src/shoelace.ts

Line 69 in 6761fdc
export * from './events/events.js';

While the project does not list sideeffects in package.json, imports like import type { SlSelectEvent } from "@shoelace-style/shoelace"; can virally expose the entirety of the project to bundlers.

The easiest fix is likely surfacing ./dist/events/* as a package export so that event types can be accessed externally via a path with no runtime code allowing the consumer to fix their own problems here.

Alternative options require a little testing, that I think will work out, but are less simple/guaranteed:

* add the `type` label _should_ fix this issue at the TypeScript level; (IIUC) it's absence confused the build into being unable to separate types from actual code in that import.

* add `sideeffect` listings to `package.json` to prevent vitality at the bundle level. This can be difficult to maintain over time, but is a great way to ensure consumers have the smallest amount of code built into their applications.

interesting that --verbatimModuleSyntax didn't flag this.

https://www.typescriptlang.org/tsconfig/#verbatimModuleSyntax

exposing it in the exports and then doing export type * from './events/events.js' should fix the issue since build tools can flag it.

sideEffects is a path i'd rather not even touch. Thats a path to pain.

At least that gets my vote.

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

2 participants