-
Notifications
You must be signed in to change notification settings - Fork 175
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
Markdown parser lib change, snarkdown instead of marked. Build tests with babel instead of ts-node. #873
Conversation
size-limit report 📦
|
✅ Deployed successfully to: https://turnilo-marked-gmbbyye42a-ew.a.run.app |
✅ Deployed successfully to: https://turnilo-marked-gmbbyye42a-ew.a.run.app |
✅ Deployed successfully to: https://turnilo-marked-gmbbyye42a-ew.a.run.app |
✅ Deployed successfully to: https://turnilo-marked-gmbbyye42a-ew.a.run.app |
✅ Deployed successfully to: https://turnilo-marked-gmbbyye42a-ew.a.run.app |
✅ Deployed successfully to: https://turnilo-marked-gmbbyye42a-ew.a.run.app |
if (!extendedDescription) { | ||
return <MarkdownNode markdown={description || STRINGS.noDescription} />; | ||
return <React.Suspense fallback={Loader}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh my!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works and how!
src/client/drag-and-drop-polyfill.ts
Outdated
// @ts-ignore | ||
import("../../lib/polyfill/drag-drop-polyfill.css") | ||
import(/* webpackChunkName: "dnd-css" */ "../../lib/polyfill/drag-drop-polyfill.css") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume these comments are meaningful for webpack? Will something horribly broke if someone removes them by accident?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It sets name for new chunk, otherwise webpack chunk index number as its name. It should be good without thoes comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move button to Suspense boundary.
Done 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Conditional approve. Code is good, let's discuss with team if UX is acceptable.
✅ Deployed successfully to: https://turnilo-marked-gmbbyye42a-ew.a.run.app |
✅ Deployed successfully to: https://turnilo-marked-gmbbyye42a-ew.a.run.app |
✅ Deployed successfully to: https://turnilo-marked-gmbbyye42a-ew.a.run.app |
882391f
to
5c982f6
Compare
✅ Deployed successfully to: https://turnilo-marked-gmbbyye42a-ew.a.run.app |
✅ Deployed successfully to: https://turnilo-marked-gmbbyye42a-ew.a.run.app |
test/setup/mocha.js
Outdated
extensions: [".ts", ".tsx", ".js", ".jsx"], | ||
ignore: [ | ||
function(filePath) { | ||
return filePath.includes("node_modules") && !filePath.includes("snarkdown") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By default ignore
is like [/node_modules/]
. In the meantime, ESM module was imported under snarkdown
and I had to treat it with a transpilation. But I re-checked this setting and it seems it's now all good without it 🤔
✅ Deployed successfully to: https://turnilo-marked-gmbbyye42a-ew.a.run.app |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yay!
✅ Deployed successfully to: https://turnilo-marked-gmbbyye42a-ew.a.run.app |
#845
https://bundlephobia.com/package/marked@4.0.12 vs https://bundlephobia.com/package/snarkdown@2.0.0