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

feat: add LaTeX to Markdown #1734

Merged
merged 9 commits into from
Feb 8, 2024
Merged

feat: add LaTeX to Markdown #1734

merged 9 commits into from
Feb 8, 2024

Conversation

wusteven815
Copy link
Contributor

@wusteven815 wusteven815 commented Jan 18, 2024

  • Adds Add remark-math plugin to MarkdownEditor #1720
    • Updated react-markdown to 8.0.7
    • Updated remark-gfm to 4.0.0
    • Installed rehype-katex 6.0.3
    • Installed remark-math 5.1.1
    • Some of these are not the latest versions to match compatibility (with React 17)
  • Updated Jest config for pure ESM libraries (thanks to @mattrunyon)

@wusteven815 wusteven815 self-assigned this Jan 18, 2024
@wusteven815
Copy link
Contributor Author

A few notes:

  • import 'katex/dist/katex.min.css'; is needed, but ESLint is not happy ('katex' should be listed in the project's dependencies. Run 'npm i -S katex' to add it (import/no-extraneous-dependencies))
    • Not sure if there will be any weird behaviours from being in a monorepo
  • The following equation (from the issue) does not work
$$\eqalign{
(a+b)^2 &= (a+b)(a+b) \\
&= a^2 + ab + ba + b^2 \\
&= a^2 + 2ab + b^2
}$$

@wusteven815 wusteven815 linked an issue Jan 19, 2024 that may be closed by this pull request
@mofojed mofojed requested a review from mattrunyon January 22, 2024 15:01
@mofojed
Copy link
Member

mofojed commented Jan 22, 2024

Check how big the katex.css file is, Don is concerned about size of loading it up front.

@wusteven815
Copy link
Contributor Author

Check how big the katex.css file is, Don is concerned about size of loading it up front.

It's 23KB

@mofojed
Copy link
Member

mofojed commented Jan 22, 2024

@dsmmcken at 23kb I wouldn't be concerned about the extra weight

@mattrunyon
Copy link
Collaborator

For the linting issue, add katex to the package.json of the package that needs it, then run npm install from the root. Might need to restart VSCode or hit ctrl+shift+p (cmd on mac) and select Restart ESLint Server for eslint to recognize the update locally

You can also do npm install katex -w @deephaven/dashboard-core-plugins from the root to install it to the package

@mattrunyon
Copy link
Collaborator

Replace jest.config.base.cjs contents with this to get the tests to work

const path = require('path');

// List of node_modules that need to be transformed from ESM to CJS for jest to work
const nodeModulesToTransform = [
  // monaco
  'monaco-editor',
  // plotly.js dependencies
  'd3-interpolate',
  'd3-color',
  // react-markdown and its dependencies
  'react-markdown',
  'vfile',
  'vfile-message',
  'unist-util.*',
  'unified',
  'bail',
  'is-plain-obj',
  'trough',
  'remark.*',
  'mdast-util.*',
  'micromark.*',
  'decode-named-character-reference',
  'trim-lines',
  'property-information',
  'hast-util.*',
  '.*separated-tokens',
  'ccount',
  'devlop',
  'escape-string-regexp',
  'markdown-table',
  'zwitch',
  'longest-streak',
  'rehype.*',
  'web-namespaces',
  'hastscript',
];

module.exports = {
  transform: {
    '.(ts|tsx|js|jsx)': [
      'babel-jest',
      {
        rootMode: 'upward',
        plugins: ['@deephaven/babel-preset/mockCssImportPlugin'],
      },
    ],
  },
  // Makes Jest transform some node_modules when needed. Usually because they are pure ESM and Jest needs CJS
  // By default, Jest ignores transforming node_modules
  // When switching to transform all of node_modules, it caused a babel error
  transformIgnorePatterns: [
    `node_modules/(?!(${nodeModulesToTransform.join('|')})/)`,
  ],
  moduleNameMapper: {
    '\\.(css|less|scss|sass)$': 'identity-obj-proxy',
    '\\.(jpg|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga)$':
      path.join(__dirname, './__mocks__/fileMock.js'),
    '^fira$': 'identity-obj-proxy',
    '^monaco-editor$': path.join(
      __dirname,
      'node_modules',
      'monaco-editor/esm/vs/editor/editor.api.js'
    ),
    // Handle monaco worker files
    '\\.worker.*$': 'identity-obj-proxy',
    // All packages except icons use src code
    '^@deephaven/(?!icons)(.*)$': path.join(__dirname, './packages/$1/src'),
  },
  testEnvironment: 'jsdom',
  setupFilesAfterEnv: [path.join(__dirname, './jest.setup.ts')],
};

@wusteven815 wusteven815 marked this pull request as ready for review January 29, 2024 16:31
@wusteven815 wusteven815 marked this pull request as draft January 29, 2024 16:33
Copy link

codecov bot commented Jan 29, 2024

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (629ed23) 46.48% compared to head (34a1126) 46.07%.
Report is 28 commits behind head on main.

❗ Current head 34a1126 differs from pull request most recent head e3c9d86. Consider uploading reports for the commit e3c9d86 to get more accurate results

Files Patch % Lines
...ashboard-core-plugins/src/panels/MarkdownPanel.tsx 44.44% 4 Missing and 1 partial ⚠️
...ashboard-core-plugins/src/panels/NotebookPanel.tsx 25.00% 3 Missing ⚠️
...e-plugins/src/controls/markdown/MarkdownEditor.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1734      +/-   ##
==========================================
- Coverage   46.48%   46.07%   -0.42%     
==========================================
  Files         617      627      +10     
  Lines       37289    37625     +336     
  Branches     9378     9470      +92     
==========================================
+ Hits        17335    17336       +1     
- Misses      19900    20235     +335     
  Partials       54       54              
Flag Coverage Δ
unit 46.07% <35.71%> (-0.42%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@wusteven815 wusteven815 marked this pull request as ready for review January 29, 2024 17:26
@wusteven815 wusteven815 requested a review from mofojed February 5, 2024 16:14
Copy link
Collaborator

@mattrunyon mattrunyon left a comment

Choose a reason for hiding this comment

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

Due to our Vite config, the lazy loading doesn't actually offload much (only about 2.5kB). The vendor bundle increases from 792kB gzipped to 1417kB gzipped

I'll see if we can fix this. There was some reason we manually specified chunks and I think it was to split out monaco and plotly. I believe there was also some memory usage issue w/ the source maps and everything in 1 giant chunk

// plotly.js dependencies
'd3-interpolate',
'd3-color',
// react-markdown and its dependencies
Copy link
Member

Choose a reason for hiding this comment

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

@mattrunyon this looks like it was fun to figure out

@wusteven815 wusteven815 merged commit 434930a into deephaven:main Feb 8, 2024
4 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Feb 8, 2024
@wusteven815 wusteven815 deleted the feat-1720-latex branch February 9, 2024 15:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add remark-math plugin to MarkdownEditor
3 participants