fix(content): fix deprecation warnings for marked package #487

luishcastroc · 2023-06-26T21:13:22Z

Fixed by adding the new extensions as well added a test

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/analogjs/analog/blob/main/CONTRIBUTING.md#-commit-message-guidelines
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

Which package are you modifying?

What is the current behavior?

When serving the application and prerendering routes, a console warning is shown for the marked package

marked(): highlight and langPrefix parameters are deprecated since version 5.0.0, should not be used and will be removed in the future. Instead use https://www.npmjs.com/package/marked-highlight.
marked(): mangle parameter is enabled by default, but is deprecated since version 5.0.0, and will be removed in the future. To clear this warning, install https://www.npmjs.com/package/marked-mangle, or disable by setting `{mangle: false}`.
marked(): headerIds and headerPrefix parameters enabled by default, but are deprecated since version 5.0.0, and will be removed in the future. To clear this warning, install  https://www.npmjs.com/package/marked-gfm-heading-id, or disable by setting `{headerIds: false}`.

Closes #482

What is the new behavior?

After the fix no warning should appear.

Does this PR introduce a breaking change?

Yes
No

Other information

[optional] What gif best describes this PR or how it makes you feel?

Fixed by adding the new extensions as well added a test

netlify · 2023-06-26T21:13:27Z

✅ Deploy Preview for analog-blog ready!

Name	Link
🔨 Latest commit	`1afffc3`
🔍 Latest deploy log	https://app.netlify.com/sites/analog-blog/deploys/649cd475331aee000702ed2c
😎 Deploy Preview	https://deploy-preview-487--analog-blog.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify · 2023-06-26T21:13:29Z

✅ Deploy Preview for analog-docs ready!

Name	Link
🔨 Latest commit	`1afffc3`
🔍 Latest deploy log	https://app.netlify.com/sites/analog-docs/deploys/649cd475b82e8c00093ceb3f
😎 Deploy Preview	https://deploy-preview-487--analog-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify · 2023-06-26T21:13:30Z

✅ Deploy Preview for analog-app ready!

Name	Link
🔨 Latest commit	`1afffc3`
🔍 Latest deploy log	https://app.netlify.com/sites/analog-app/deploys/649cd475bb1b3a00088a9f16
😎 Deploy Preview	https://deploy-preview-487--analog-app.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

luishcastroc · 2023-06-26T21:15:10Z

#482 Hope this works 🥇

brandonroberts · 2023-06-27T13:28:39Z

Thanks @luishcastroc! Things aren't rendering correctly now comparing the deploy preview against the current version

https://deploy-preview-487--analog-blog.netlify.app/blog/2022-12-31-my-second-post
https://analog-blog.netlify.app/blog/2022-12-31-my-second-post

If you go to https://deploy-preview-487--analog-blog.netlify.app/blog and click "My Second Post", it doesn't show the code snippet. If you refresh the page, it shows correctly.

The e2e test failure is unrelated so don't worry about that for now.

luishcastroc · 2023-06-27T13:30:26Z

Thanks @luishcastroc! Things aren't rendering correctly now comparing the deploy preview against the current version

https://deploy-preview-487--analog-blog.netlify.app/blog/2022-12-31-my-second-post https://analog-blog.netlify.app/blog/2022-12-31-my-second-post

If you go to https://deploy-preview-487--analog-blog.netlify.app/blog and click "My Second Post", it doesn't show the code snippet. If you refresh the page, it shows correctly.

The e2e test failure is unrelated so don't worry about that for now.

Thanks i really didn't know how to test it properly so i gave it a shot but with that i can now tell for sure, let me give it a look

applying new approach on marked to replace setOptions for use this might need to be replaced once the marked/types are updated

luishcastroc · 2023-06-28T16:35:37Z

@brandonroberts i think it should be fixed now however e2e seems to be still failing.

brandonroberts · 2023-06-28T21:18:49Z

@luishcastroc works as expected!

Question: What do you think about turning the Marked object into an Injectable service? Like a marked.service.ts class that's provided/injected into the MarkdownContentRenderService internally?

It would still behave like a singleton.

luishcastroc · 2023-06-28T21:49:17Z

@luishcastroc works as expected!

Question: What do you think about turning the Marked object into an Injectable service? Like a marked.service.ts class that's provided/injected into the MarkdownContentRenderService internally?

It would still behave like a singleton.

i was thinking on this however since all the logic was inside the service i just complemented with the new approach, i think since this is an angular ecosystem it definitely makes sense so let me do the proper changes.

thanks for the feedback.

luishcastroc · 2023-06-28T23:32:59Z

@luishcastroc works as expected!

Question: What do you think about turning the Marked object into an Injectable service? Like a marked.service.ts class that's provided/injected into the MarkdownContentRenderService internally?

It would still behave like a singleton.

@brandonroberts i sent a commit with the markedService injected in root but i wanted to know in your experience if that would be the best approach (seems to be straight forward after that). another one i tried was this.

export function withMarkdownRenderer(): Provider {
  return {
    provide: ContentRenderer,
    useClass: MarkdownContentRendererService,
    deps: [MarkedSetupService],
  };
}

export function provideContent(...features: Provider[]) {
  return [...features, MarkedSetupService];
}

not sure if that would be the best

brandonroberts · 2023-06-28T23:43:12Z

@luishcastroc I think it should follow the same pattern as the provider and not use providedIn: 'root'. That way if you want to lazy load the provideContent you can

brandonroberts

LGTM

brandonroberts · 2023-06-29T01:39:18Z

Thanks for pushing through @luishcastroc! We'll need to add a migration for the new dependencies but that can be done separately.

brandonroberts · 2023-06-29T01:41:09Z

@allcontributors add @luishcastroc for code

allcontributors · 2023-06-29T01:41:19Z

@brandonroberts

I've put up a pull request to add @luishcastroc! 🎉

luishcastroc · 2023-07-02T23:09:57Z

Thanks for pushing through @luishcastroc! We'll need to add a migration for the new dependencies but that can be done separately.

let me read a little bit about how to do this and I'll take a look

fix(content): fix deprecation warnings for marked package

bdd90b0

Fixed by adding the new extensions as well added a test

luishcastroc added 4 commits June 27, 2023 21:00

fix(content): fix deprecation warnings and apply new approach

6e870f1

applying new approach on marked to replace setOptions for use this might need to be replaced once the marked/types are updated

chore(content): add angular service for marked setup

6a655b2

chore(content): change approach for providing the marked service

c1aaaea

chore(content): fix unit test for markdown renderer service

1afffc3

brandonroberts approved these changes Jun 29, 2023

View reviewed changes

brandonroberts merged commit 87a978b into analogjs:main Jun 29, 2023

allcontributors bot mentioned this pull request Jun 29, 2023

docs: add luishcastroc as a contributor for code #489

Merged

brandonroberts mentioned this pull request Jun 29, 2023

Feature: add migration to content package for new marked dependencies. #490

Closed

Villanuevand pushed a commit to Villanuevand/analog that referenced this pull request Sep 12, 2023

fix(content): fix deprecation warnings for marked package (analogjs#487)

bbd2789

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(content): fix deprecation warnings for marked package #487

fix(content): fix deprecation warnings for marked package #487

luishcastroc commented Jun 26, 2023 •

edited by brandonroberts

Loading

netlify bot commented Jun 26, 2023 •

edited

Loading

netlify bot commented Jun 26, 2023 •

edited

Loading

netlify bot commented Jun 26, 2023 •

edited

Loading

luishcastroc commented Jun 26, 2023 •

edited

Loading

brandonroberts commented Jun 27, 2023

luishcastroc commented Jun 27, 2023

luishcastroc commented Jun 28, 2023

brandonroberts commented Jun 28, 2023

luishcastroc commented Jun 28, 2023

luishcastroc commented Jun 28, 2023

brandonroberts commented Jun 28, 2023

brandonroberts left a comment

brandonroberts commented Jun 29, 2023

brandonroberts commented Jun 29, 2023

allcontributors bot commented Jun 29, 2023

luishcastroc commented Jul 2, 2023

fix(content): fix deprecation warnings for marked package #487

fix(content): fix deprecation warnings for marked package #487

Conversation

luishcastroc commented Jun 26, 2023 • edited by brandonroberts Loading

PR Checklist

PR Type

Which package are you modifying?

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

[optional] What gif best describes this PR or how it makes you feel?

netlify bot commented Jun 26, 2023 • edited Loading

✅ Deploy Preview for analog-blog ready!

netlify bot commented Jun 26, 2023 • edited Loading

✅ Deploy Preview for analog-docs ready!

netlify bot commented Jun 26, 2023 • edited Loading

✅ Deploy Preview for analog-app ready!

luishcastroc commented Jun 26, 2023 • edited Loading

brandonroberts commented Jun 27, 2023

luishcastroc commented Jun 27, 2023

luishcastroc commented Jun 28, 2023

brandonroberts commented Jun 28, 2023

luishcastroc commented Jun 28, 2023

luishcastroc commented Jun 28, 2023

brandonroberts commented Jun 28, 2023

brandonroberts left a comment

Choose a reason for hiding this comment

brandonroberts commented Jun 29, 2023

brandonroberts commented Jun 29, 2023

allcontributors bot commented Jun 29, 2023

luishcastroc commented Jul 2, 2023

luishcastroc commented Jun 26, 2023 •

edited by brandonroberts

Loading

netlify bot commented Jun 26, 2023 •

edited

Loading

netlify bot commented Jun 26, 2023 •

edited

Loading

netlify bot commented Jun 26, 2023 •

edited

Loading

luishcastroc commented Jun 26, 2023 •

edited

Loading