-
-
Notifications
You must be signed in to change notification settings - Fork 180
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
chore: migrate to standalone #476
Closed
Closed
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
b77382b
chore: update to marked version 9
robertIsaac 87fbfbe
feat: add `provideMarkdown` as a new way to configure ngx-markdown
robertIsaac 02e3143
chore: run ng generate @angular/core:standalone
robertIsaac 2c42b7c
chore: run ng generate @angular/core:standalone
robertIsaac 8dc788f
chore: move routing to the new standalone
robertIsaac File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
import { Routes } from '@angular/router'; | ||
|
||
export const appRoutes: Routes = [ | ||
{ | ||
path: 'get-started', | ||
loadComponent: () => import('./get-started/get-started.component'), | ||
data: { label: 'Get Started' }, | ||
}, | ||
{ | ||
path: 'cheat-sheet', | ||
loadComponent: () => import('./cheat-sheet/cheat-sheet.component'), | ||
data: { label: 'Cheat Sheet' }, | ||
}, | ||
{ | ||
path: 'syntax-highlight', | ||
loadComponent: () => import('./syntax-highlight/syntax-highlight.component'), | ||
data: { label: 'Syntax Highlight' }, | ||
}, | ||
{ | ||
path: 'bindings', | ||
loadComponent: () => import('./bindings/bindings.component'), | ||
data: { label: 'Bindings' }, | ||
}, | ||
{ | ||
path: 'plugins', | ||
loadComponent: () => import('./plugins/plugins.component'), | ||
data: { label: 'Plugins' }, | ||
}, | ||
{ | ||
path: 're-render', | ||
loadComponent: () => import('./rerender/rerender.component'), | ||
data: { label: 'Re-render' }, | ||
}, | ||
{ | ||
path: '**', | ||
redirectTo: 'get-started', | ||
}, | ||
]; |
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
import { provideHttpClient, withInterceptorsFromDi } from '@angular/common/http'; | ||
import { ApplicationConfig, SecurityContext } from '@angular/core'; | ||
import { provideAnimations } from '@angular/platform-browser/animations'; | ||
import { provideRouter, withInMemoryScrolling } from '@angular/router'; | ||
import { gfmHeadingId } from 'marked-gfm-heading-id'; | ||
import { ClipboardOptions, MARKDOWN_EXTENSIONS, MarkedOptions } from 'ngx-markdown'; | ||
import { provideMarkdown } from 'ngx-markdown'; | ||
import { markedOptionsFactory } from '@app/marked-options-factory'; | ||
import { AnchorService } from '@shared/anchor/anchor.service'; | ||
import { ClipboardButtonComponent } from '@shared/clipboard-button'; | ||
import { appRoutes } from './app-routes'; | ||
|
||
export const appConfig: ApplicationConfig = { | ||
providers: [ | ||
provideMarkdown({ | ||
markedOptions: { | ||
provide: MarkedOptions, | ||
useFactory: markedOptionsFactory, | ||
deps: [AnchorService], | ||
}, | ||
clipboardOptions: { | ||
provide: ClipboardOptions, | ||
useValue: { | ||
buttonComponent: ClipboardButtonComponent, | ||
}, | ||
}, | ||
sanitize: SecurityContext.NONE, | ||
}), | ||
provideRouter(appRoutes, withInMemoryScrolling({scrollPositionRestoration: 'enabled', anchorScrolling: 'enabled'})), | ||
{ | ||
provide: MARKDOWN_EXTENSIONS, | ||
useValue: [gfmHeadingId()], | ||
}, | ||
provideAnimations(), | ||
provideHttpClient(withInterceptorsFromDi()), | ||
], | ||
}; |
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
For this injection token, one option that's slightly more aligned with the new
provide*
function pattern, is to use awith*
function that you pass in as a second parameter to theprovide
function, which allows users to add additional options like this use case. So, for example, a user could do this in theirapp.config
The
withExtensions
function would set up the appropriate injection token for theprovideMarkdown
to use. You can see an example of this in the article I linked (searchwithColor
): https://www.angulararchitects.io/en/blog/patterns-for-custom-standalone-apis-in-angular/Angular's new
provideRouter
function has severalwith*
feature functions, if you want to use that for inspiration as well. E.g.withPreloading
https://angular.io/api/router/withPreloadingYou could potentially do the same with several of the optional features that are embedded in the Markdown config, like moving Clipboard options into a
withClipboard
feature function. But that may be a bit more of an overhaul than @jfcere may be comfortable with?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 think we should be consistent
Either to add the extensions to the configuration interface
Or to use with for everything
What do you think @jfcere
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.
The main difference with the injection token, though, is that it's not integrated into the config interface right now. So it's already sort of its own thing, and could be converted to a
with
function. But I agree, it would be nice to lean all the way into that pattern for optional features.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 agree that it should be consistent, honestly I never had time to really dive into the standalone components architecture so I am not sure which approach is best.
I always favored having providers within the
markdownModuleConfig
so that it kinda regroup everything that is used by the ngx-markdown library instead of adding providers in theproviders
property of the module and ending up with a bunch of stuff you are not clearly sure it is used by what module.That being said, I do think that the
MARKDOWN_EXTENSIONS
should be an option within themarkdownModuleConfig
but this should be a comment for the previous PR #474. So I would be confortable with not having a bunch ofwithSomething
functions for now.Does it make sense to you guys?
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 agree
I will do it right now
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.
@jfcere I totally agree on having the self-contained config vs having a bunch of separate providers. But I feel like, when using the new
provide*
function paradigm, thewith*
feature functions feel a bit more natural, than passing in a large config to the function. So, for example, rather than having thisyou could have something like this
Internally those functions could still create the same config object, but it would remove the burden from the user to have to construct it. I don't have a strong preference, but the latter feels a bit more ergonomic and in line with the new direction the framework is going with their APIs.
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.
So taking the exemple...
You would end up with 4
with*
functions as such (one for each property of theMarkdownConfig
) ?Or this could probably be in the middle such as...
I am an OCD person where having a bunch of
with*
functions doesn't seem like an absolute necessity and I would be comfortable with the last one, I think it's pretty straightforward and it does remove the burden on the user to construct the whole provider object (as this could be done internally).What you guys think?
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.
Well, I was thinking the core
MarkedOptions
would be the first parameter of theprovideMarkdown
function, since those are core functionality. And then the other types of feature options would be encapsulated bywith
functions.e.g.
It's just making a slight delineation between core function and additional features. But the other way could work too.