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

Create non-auto-registering routes #1450

Merged
merged 57 commits into from
Jul 24, 2023
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
7bb4caa
initial attempt at not auto defining
KonnorRogers Jul 7, 2023
bb01ecd
add files with -
KonnorRogers Jul 7, 2023
2a514c3
continued work on removing auto-define
KonnorRogers Jul 7, 2023
2dcbf70
fix component definitions
KonnorRogers Jul 10, 2023
8e94da3
update with new tag stuff
KonnorRogers Jul 10, 2023
6902ced
fix lots of things
KonnorRogers Jul 10, 2023
746cd0a
fix improper scoped elements
KonnorRogers Jul 10, 2023
d6d42ce
working through side effects
KonnorRogers Jul 11, 2023
6b8b43e
continued react wrapper work
KonnorRogers Jul 12, 2023
313c538
update changelog
KonnorRogers Jul 12, 2023
0efbeba
fix conflicts
KonnorRogers Jul 12, 2023
c71e6f8
formatting
KonnorRogers Jul 13, 2023
206e3a9
fixes
KonnorRogers Jul 13, 2023
b76ba9e
update changelog
KonnorRogers Jul 13, 2023
3c37755
lint / formatting
KonnorRogers Jul 13, 2023
f741b57
fix version injection
KonnorRogers Jul 13, 2023
782de18
fix version injection, work on test
KonnorRogers Jul 13, 2023
bbf5bb4
fix version injection, work on test
KonnorRogers Jul 13, 2023
b6bcc6e
Merge branch 'next' into konnorrogers/dont-auto-define
KonnorRogers Jul 14, 2023
7527151
fix merge conflicts
KonnorRogers Jul 14, 2023
165d76e
fix jsdoc null issue
KonnorRogers Jul 14, 2023
9d746c6
fix templates
KonnorRogers Jul 14, 2023
47104bf
use exports
KonnorRogers Jul 14, 2023
5673819
working on tests
KonnorRogers Jul 14, 2023
28462da
working on registration mocking
KonnorRogers Jul 15, 2023
de8a28a
fix customElements test
KonnorRogers Jul 15, 2023
9230347
linting
KonnorRogers Jul 15, 2023
86246bc
fix some test stuff
KonnorRogers Jul 15, 2023
c5e2195
clean up test
KonnorRogers Jul 15, 2023
af4780a
clean up comment
KonnorRogers Jul 15, 2023
682f74d
rename scopedElements to dependencies
KonnorRogers Jul 18, 2023
206e1d3
linting / formatting
KonnorRogers Jul 18, 2023
bb6858e
Merge branch 'next' of https://github.com/shoelace-style/shoelace int…
KonnorRogers Jul 18, 2023
8ffb396
linting / formatting
KonnorRogers Jul 18, 2023
9af7ee5
mark all packages external and still bundle
KonnorRogers Jul 18, 2023
f23290e
set bundle false
KonnorRogers Jul 19, 2023
27c8574
set bundle true
KonnorRogers Jul 19, 2023
09182fb
dont minify
KonnorRogers Jul 19, 2023
64bd313
Merge branch 'next' of https://github.com/shoelace-style/shoelace int…
KonnorRogers Jul 19, 2023
4bb7126
fix merge conflicts
KonnorRogers Jul 19, 2023
1329599
use built shoelace-element
KonnorRogers Jul 19, 2023
a46cca0
fix lint errors
KonnorRogers Jul 19, 2023
8799888
prettier
KonnorRogers Jul 19, 2023
02d29f7
appease eslint
KonnorRogers Jul 19, 2023
070f411
appease eslint gods
KonnorRogers Jul 19, 2023
8a8f3f5
appease eslint gods
KonnorRogers Jul 19, 2023
f35cb70
appease eslint gods
KonnorRogers Jul 19, 2023
48168e8
appease eslint gods
KonnorRogers Jul 19, 2023
bbce79d
add shoelace-autoloader
KonnorRogers Jul 19, 2023
23f4166
move it all into 1 function
KonnorRogers Jul 19, 2023
1fb8a78
add exportmaps note
KonnorRogers Jul 19, 2023
d0c4d5d
Merge branch 'next' of https://github.com/shoelace-style/shoelace int…
KonnorRogers Jul 20, 2023
676ec88
prettier
KonnorRogers Jul 20, 2023
20f9340
add jsdelivr entrypoint
KonnorRogers Jul 20, 2023
f8fadc5
read as utf8
KonnorRogers Jul 24, 2023
2eab6f9
update docs with .component.js importS
KonnorRogers Jul 24, 2023
138c0ad
prettier
KonnorRogers Jul 24, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 31 additions & 1 deletion custom-elements-manifest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { parse } from 'comment-parser';
import { pascalCase } from 'pascal-case';
import commandLineArgs from 'command-line-args';
import fs from 'fs';
import * as path from 'path'

const packageData = JSON.parse(fs.readFileSync('./package.json', 'utf8'));
const { name, description, version, author, homepage, license } = packageData;
Expand All @@ -26,7 +27,7 @@ function replace(string, terms) {
}

export default {
globs: ['src/components/**/*.ts'],
globs: ['src/components/**/*.component.ts'],
exclude: ['**/*.styles.ts', '**/*.test.ts'],
plugins: [
// Append package data
Expand All @@ -36,7 +37,33 @@ export default {
customElementsManifest.package = { name, description, version, author, homepage, license };
}
},
// Infer tag names because we no longer use @customElement decorators.
{
name: "shoelace-infer-tag-names",
analyzePhase({ ts, node, moduleDoc }) {
switch (node.kind) {
case ts.SyntaxKind.ClassDeclaration: {
const className = node.name.getText();
const classDoc = moduleDoc?.declarations?.find(declaration => declaration.name === className);

const importPath = moduleDoc.path

// This is kind of a best guess at components. "thing.component.ts"
if (!importPath.endsWith(".component.ts")) {
return
}

const tagName = "sl-" + path.basename(importPath, ".component.ts")


classDoc.tagName = tagName

// This used to be set to true by @customElement
classDoc.customElement = true
}
}
}
},
// Parse custom jsDoc tags
{
name: 'shoelace-custom-tags',
Expand All @@ -58,6 +85,9 @@ export default {
});
});

// This is what allows us to map JSDOC comments to ReactWrappers.
classDoc["jsDoc"] = node.jsDoc.map((jsDoc) => jsDoc.getFullText()).join("\n")

const parsed = parse(`${customComments}\n */`);
parsed[0].tags?.forEach(t => {
switch (t.tag) {
Expand Down
7 changes: 6 additions & 1 deletion docs/pages/resources/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ New versions of Shoelace are released as-needed and generally occur when a criti

## Next

- Added JSDOC comments to React Wrappers for better documentation when hovering a component. []
KonnorRogers marked this conversation as resolved.
Show resolved Hide resolved
- Added `displayName` to React Wrappers for better debugging. []
- Split auto-defining routes and non-auto-defining routes for Components. []
- Fixed React Treeshaking by introducing `sideEffects` key in `package.json`. []
KonnorRogers marked this conversation as resolved.
Show resolved Hide resolved
- Fixed a bug in `<sl-tree>` where it was auto-defining `<sl-tree-item>`. []
- Added tests for `<sl-qr-code>` [#1416]
- Added support for pressing [[Space]] to select/toggle selected `<sl-menu-item>` elements [#1429]
- Fixed a bug in focus trapping of modal elements like `<sl-dialog>`. We now manually handle focus ordering as well as added `offsetParent()` check for tabbable boundaries in Safari. Test cases added for `<sl-dialog>` inside a shadowRoot [#1403]
Expand Down Expand Up @@ -1591,4 +1596,4 @@ The following pages demonstrate why this change was necessary.

## 2.0.0-beta.1

- Initial release
- Initial release
3 changes: 1 addition & 2 deletions docs/pages/resources/contributing.md
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,6 @@ Then use the following syntax for comments so they appear in the generated docs.
* @cssproperty --color: The component's text color.
* @cssproperty --background-color: The component's background color.
*/
@customElement('sl-example')
export default class SlExample {
// ...
}
Expand Down Expand Up @@ -467,4 +466,4 @@ Guidelines for writing tests:
- Tests should not produce log lines. Note that sometimes this cannot be prevented as the test runner might log errors (e.g. 404s).
- Try keeping the main test readable: Extract more complicated sets of selectors/commands/assertions into separate functions.
- Try to aim testing the user facing features of the component instead of the internal workings of the component.
- Group multiple tests for one feature into describe blocks.
- Group multiple tests for one feature into describe blocks.
10 changes: 10 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,15 @@
"web-types": "dist/web-types.json",
"type": "module",
"types": "dist/shoelace.d.ts",
"sideEffects": [
"./dist/shoelace.js",
"./dist/shoelace-autoloader.js",
"./dist/components/**/*.js",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks for your work, but the sideEffects settings here seems to break the cherry picking usage such as:

import '@shoelace-style/shoelace/dist/components/button/button.js';

Since the dist/components/button/button.js also imports dist/chunks/*.js which are not marked as sideEffects, which will be tree-shaked in production mode.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a repo by chance?

What bundler are you using? I tested mainly with Vite, Webpack (via CRA), and Parcel.

In Stackblitz using NPM + Vite, it seems to work.

https://stackblitz.com/edit/vitejs-vite-42iegf?file=main.js,package.json,index.html&terminal=dev

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm using Webpack. But your demo is just ok to reproduce it. Try npm run build && npm run preview. Tree-shaking only happens in production mode by default, so npm run dev is not the case.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also the docs for 2.6.0 version suggest to import for bundler like so import '@shoelace-style/shoelace/dist/components/button/button.component.js'; but by doing this you also break the dev environment.

Using the same demo @KonnorRogers provided I've just changed the import for the button.

https://stackblitz.com/edit/vitejs-vite-n3cvck?file=main.js

Not sure if this is related to the problem or it's a docs error. @claviska

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Antony92 Good call on the new import docs. Its probably pulling from the manifest 🤔

Copy link
Collaborator Author

@KonnorRogers KonnorRogers Aug 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Antony92 🤔 where in the docs does it say to use the .component.js import? There's a note about auto-registering, but thats different.

"./dist/translations/**/*.*",
"./src/translations/**/*.*",
"// COMMENT: This monstrosity below isn't perfect, but its like 99% to get bundlers to recognize 'thing.component.ts' as having no side effects. Example: https://regexr.com/7grof",
"./dist/components/**/*((?<!(\\.component|\\.styles)))\\.js"
],
"exports": {
".": {
"types": "./dist/shoelace.d.ts",
Expand All @@ -20,6 +29,7 @@
"./dist/utilities/*": "./dist/utilities/*",
"./dist/react": "./dist/react/index.js",
"./dist/react/*": "./dist/react/*",
"./dist/shoelace.js": "./dist/shoelace.js",
"./dist/translations/*": "./dist/translations/*"
},
"files": [
Expand Down
7 changes: 5 additions & 2 deletions scripts/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import getPort, { portNumbers } from 'get-port';
import ora from 'ora';
import util from 'util';
import * as path from 'path';
import { readFileSync } from 'fs';

const { serve } = commandLineArgs([{ name: 'serve', type: Boolean }]);
const outdir = 'dist';
Expand All @@ -22,6 +23,7 @@ let childProcess;
let buildResults;

const bundleDirectories = [cdndir, outdir];
const shoelaceVersion = JSON.stringify(JSON.parse(readFileSync(path.join(process.cwd(), "package.json")).toString()).version.toString())
KonnorRogers marked this conversation as resolved.
Show resolved Hide resolved

//
// Runs 11ty and builds the docs. The returned promise resolves after the initial publish has completed. The child
Expand Down Expand Up @@ -97,7 +99,8 @@ async function buildTheSource() {
chunkNames: 'chunks/[name].[hash]',
define: {
// Floating UI requires this to be set
'process.env.NODE_ENV': '"production"'
'process.env.NODE_ENV': '"production"',
'shoelaceVersion': shoelaceVersion,
},
bundle: true,
//
Expand All @@ -108,7 +111,7 @@ async function buildTheSource() {
//
external: alwaysExternal,
splitting: true,
plugins: []
plugins: [],
KonnorRogers marked this conversation as resolved.
Show resolved Hide resolved
};

const npmConfig = {
Expand Down
26 changes: 23 additions & 3 deletions scripts/make-react.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ components.map(component => {

fs.mkdirSync(componentDir, { recursive: true });

const jsDoc = component.jsDoc

const source = prettier.format(
`
import * as React from 'react';
Expand All @@ -45,14 +47,32 @@ components.map(component => {
${eventNameImport}
${eventImports}

export default createComponent({
tagName: '${component.tagName}',
const tagName = '${component.tagName}'

const component = createComponent({
tagName,
elementClass: Component,
react: React,
events: {
${events}
},
displayName: "${component.name}"
})

${jsDoc}
class SlComponent extends React.Component<Parameters<typeof component>[0]> {
constructor (...args: Parameters<typeof component>) {
super(...args)
Component.define(tagName)
}
});

render () {
const { children, ...props } = this.props
return React.createElement(component, props, children)
}
}

export default SlComponent;
`,
Object.assign(prettierConfig, {
parser: 'babel-ts'
Expand Down
10 changes: 4 additions & 6 deletions scripts/shared.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,11 @@ export function getAllComponents(metadata) {

metadata.modules.map(module => {
module.declarations?.map(declaration => {
if (declaration.customElement) {
const component = declaration;
const path = module.path;
const component = declaration;
const path = module.path;

if (component) {
allComponents.push(Object.assign(component, { path }));
}
if (component) {
allComponents.push(Object.assign(component, { path }));
}
});
});
Expand Down
Loading