-
Notifications
You must be signed in to change notification settings - Fork 3
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: upgrade all the dependencies #22
Conversation
8d1bdf0
to
f91a8b3
Compare
test/index.tsx
Outdated
expect( | ||
marp | ||
.text() | ||
.replace(/!function.*;/, '') |
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.
This replacement is to remove the polyfill code. (I believe the tests should ignore the polyfill code)
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.
Marp Core's polyfill should never output in HTML, and be controlled in Marp React. We've already calling polyfill individually in useMarpReady()
hook (via useLayoutEffect
).
https://github.com/marp-team/marp-react/blob/master/src/hooks/marp-ready.ts
Revert change of test case, and disable script injection through script: false
in the option defined by useMarpOptions()
hook.
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.
Do you mean using <Marp markdown={markdown} options={{ script: false }} />
in the tests? Or forcing to change script
option to false
in Marp.tsx
(or marp-options.ts
) ?
=>
I've tried adding a new change of the latter approach: #22 (comment)
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.
And Marp Core peerDependency will require >= v1.0.1.
Line 93 in ab45842
"@marp-team/marp-core": ">=0.6.0", |
@@ -24,6 +24,7 @@ export default function useMarpOptions( | |||
identifier, | |||
marpOptions: { | |||
...(opts || {}), | |||
script: false, |
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 hope this change is what you wanted. (sorry if not)
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.
Yes, exactly 👍
Close #23
Use worker via CDN
doesn't work both inmaster
and this PR because the js file in the CDN is probably too old)