feat: Enable to export named component by adding component-export attribute #163
Conversation
usualoma
force-pushed
the
feat/export-named-component
branch
4 times, most recently
from
b181921
to
55a180a
Compare
usualoma
force-pushed
the
feat/export-named-component
branch
from
55a180a
to
ab23d39
Compare
|
@yusukebe Would you please review? |
src/vite/island-components.ts
Outdated
| } from '@babel/types' | ||
| import { parse as parseJsonc } from 'jsonc-parser' | ||
| // eslint-disable-next-line node/no-extraneous-import | ||
| import type { Plugin } from 'vite' | ||
|
|
||
| function addSSRCheck(funcName: string, componentName: string) { | ||
| function isComponentName(name: string) { | ||
| return /^[A-Z]/.test(name) && /[a-z]/.test(name) |
There was a problem hiding this comment.
I think the component name is /^[A-Z][A-Za-z0-9]+/. What do you think?
There was a problem hiding this comment.
I didn't explain it well enough.
I wanted to exclude capital-only exports from the island component, didn't I? What do you think?
export const LIMIT = 10;.There was a problem hiding this comment.
Well, but it is certainly better not to include _. I prefer something like this.
/^[A-Z][A-Za-z0-9]*[a-z][A-Za-z0-9]*$/I will change it to this later. Wait a minute.
There was a problem hiding this comment.
How about 130c642?
Good! I'd like to go with it.
How about writing the test? I want to have a test, though it's simple logic. But, I don't have any good idea where we should put it. Exporting isComponentName and writing test in src/vite/island-components.test.ts? Or creating src/vite/utils.ts and putting the logic into it? Do you have any thoughts?
There was a problem hiding this comment.
8365f1b added!
I think what you are trying to say is that "whether to treat it as a component or not is a very important external specification, so it should be an independent test, both to confirm the implementation and to explain it to the user".
For me, 8365f1b seems sufficient, but if I were to go further, I would suggest that it should be independent, as you say, as follows.
Or creating
src/vite/utils.tsand putting the logic into it
Opinion me, please.
There was a problem hiding this comment.
whether to treat it as a component or not is a very important external specification, so it should be an independent test, both to confirm the implementation and to explain it to the user
Yes! Exactly. Thank you for saying what I wanted to say!
8365f1b is good for me. This covers all patterns and shows the specification. Let's go with it. Sorry for taking your time.
There was a problem hiding this comment.
I wish I had been aware of this from the beginning, but I am sorry for taking up so much of your time.
There was a problem hiding this comment.
No worry! Thank you for the great PR! I'll release the new version tomorrow!
|
Thanks. I've left one comment! |
usualoma
force-pushed
the
feat/export-named-component
branch
from
2078c4e
to
130c642
Compare
|
Thank you! Let's land it. |
fixes #156
Use the
component-exportmentioned in isseu to wrap an export matching the following name as an island component.https://github.com/honojs/honox/compare/main...usualoma:honox:feat/export-named-component?expand=1#diff-dbf4bfe9bdc2ab09701d7f99d62aa69e45740919b350c8166aebe64daa3fdd54R39-R41