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: codemirror import error (#1281) #1283

Merged
merged 2 commits into from
Feb 11, 2024
Merged

Conversation

KermanX
Copy link
Member

@KermanX KermanX commented Feb 10, 2024

fix #1281

This PR fixes #1281 by partly reverts 4ef7fc9.

It is confusing that the codemirror package isn't explicitly excluded from optimizeDeps, but it is in fact not optimized (only when @slidev/cli and @slidev/client are installed from npm registry. In this monorepo, it is OK). And codemirror@5 is a UMD package, but it is then wrongly imported as an ESM package, which causes the issue. This PR re-adds codemirror related packages to optimizeDeps.include array, and import CodeMirror as default import.

I am not sure whether other packages removed from the include list by 4ef7fc9 (listed below) should be added again or not.

Lines removed in 4ef7fc9:

import { dependencies } from '../../../client/package.json'
//      optimizeDeps: {
          include: [
            ...Object.keys(dependencies).filter(i => !EXCLUDE.includes(i)),
            'codemirror/mode/javascript/javascript',
            'codemirror/mode/css/css',
            'codemirror/mode/markdown/markdown',
            'codemirror/mode/xml/xml',
            'codemirror/mode/htmlmixed/htmlmixed',
            'codemirror/addon/display/placeholder',
            'prettier/plugins/babel',
            'prettier/plugins/html',
            'prettier/plugins/typescript',
            'mermaid/dist/mermaid.esm.min.mjs',
            'mermaid/dist/mermaid.esm.mjs',
            'vite-plugin-vue-server-ref/client',
          ],

This PR re-adds these:

const INCLUDE = [
  'codemirror',
  'codemirror/mode/javascript/javascript',
  'codemirror/mode/css/css',
  'codemirror/mode/markdown/markdown',
  'codemirror/mode/xml/xml',
  'codemirror/mode/htmlmixed/htmlmixed',
  'codemirror/addon/display/placeholder',
]

@antfu
Copy link
Member

antfu commented Feb 11, 2024

The reason for removing the optimizeDeps.include is that it doesn't work without shamefully-hoist on pnpm.

Maybe we could use a runtime interop to work for both cases? Like

import * as _cm from 'codemirror'

const cm = interopDefault(_cm) as import('codemirror')

function interopDefault(mod: any) {
  return mod.default || mod
}

(I didn't test the code above)

@KermanX
Copy link
Member Author

KermanX commented Feb 11, 2024

This seems works:

import * as _CodeMirror from 'codemirror'

// eslint-disable-next-line ts/ban-ts-comment
// @ts-expect-error
const CodeMirror: typeof _CodeMirror = _CodeMirror.fromTextArea ? _CodeMirror : globalThis.CodeMirror

@antfu antfu merged commit b9d29f9 into slidevjs:main Feb 11, 2024
8 checks passed
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.

Slides cannot be modified on the side integrated editor
2 participants