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

fix(readme): add destructure import example #92

Merged
merged 1 commit into from
Nov 13, 2020
Merged

fix(readme): add destructure import example #92

merged 1 commit into from
Nov 13, 2020

Conversation

jefrydco
Copy link
Contributor

@jefrydco jefrydco commented Sep 17, 2020

Since in index.ts we don't export any default module,

export { getTheme, loadTheme, BUNDLED_THEMES } from 'shiki-themes'
export { BUNDLED_LANGUAGES } from 'shiki-languages'
export { getHighlighter } from './highlighter'
export { renderToHtml } from './renderer'
export { IThemedToken } from './themedTokenizer'

it would raise an error like this, if we use import shiki from 'shiki' syntax.

The requested module 'file:///<some-path>/node_modules/shiki/dist/index.js' does not provide an export named 'default'

I think it's better to add destructure way to import shiki.

Since in `index.ts` we don't export any default module,

https://github.com/shikijs/shiki/blob/58cb92cd9427a97d5b4f89e3cec8e12290fcb793/packages/shiki/src/index.ts#L1-L6

it would raise an error like this, if we use `import shiki from 'shiki'` syntax.

```
The requested module 'file:///mnt/Data/Projects/Github/FORKED/test-cncd-shiki/node_modules/shiki/dist/index.js' does not provide an export named 'default'
```
@octref
Copy link
Collaborator

octref commented Sep 28, 2020

Hmm, maybe I should export * as default, what do you think?

That's a bad idea: https://basarat.gitbook.io/typescript/main-1/defaultisbad.
I think it should be import * as shiki from 'shiki' though. Thanks for your contribution.

@octref octref merged commit 557d085 into shikijs:master Nov 13, 2020
@octref
Copy link
Collaborator

octref commented Nov 13, 2020

Sorry but I don't really think it clarify anything. TS users should be able to figure this out themselves so I'm removing it.

@jefrydco jefrydco deleted the patch-1 branch November 13, 2020 09:19
antfu added a commit that referenced this pull request Jan 26, 2024
Co-authored-by: Anthony Fu <anthonyfu117@hotmail.com>
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

Successfully merging this pull request may close these issues.

2 participants