-
-
Notifications
You must be signed in to change notification settings - Fork 383
feat: module replacements v3 #2068
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
base: main
Are you sure you want to change the base?
Changes from all commits
5ee3927
49dd770
f276037
e528c90
ad27978
59ac18c
6422b11
3ecebf0
7d4198b
1e94e2d
7ae1a63
8e8882b
1c72e05
a36b4ce
2464a08
d6f4b0b
83102e3
363aa2a
b30c0fd
c1f52f2
e346d83
f89546e
74eee66
a95b7a0
b60b234
7207e50
9e742ab
e365d7e
63417bf
93babd2
03584bd
375bd0a
7ad1a05
bfd7ce1
2c69f4e
e16308c
01d39ff
53aeaf6
3eeb9cf
0eda816
07bd1ba
25f99e6
55abc67
fcd16fb
c4ef9dd
f6161b8
6133408
0df272e
227f7eb
b86fbff
4041dbd
65d1a87
bcf3ed9
d1ca961
2f6de9e
503f2ed
ac522de
96accb3
46af065
12e1b56
fbed36b
2782dc7
1e5d9e3
f6838c5
f44bec6
3f1fb7e
b9ddce5
2257ca3
d80c709
b0ef6d9
e4bcf9e
4dec509
ff85c99
e2a2a69
be8378f
221e572
5145d17
50447c2
ba4c9bd
06cf980
3b01e5e
b1d49ce
ab45b57
1c35f17
64d3d68
a36d2f3
08a3a90
34082cb
d9d422a
aab96cc
02f622f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| <script setup lang="ts"> | ||
| import type { ModuleReplacement } from 'module-replacements' | ||
| import { resolveDocUrl } from 'module-replacements' | ||
|
|
||
| const props = defineProps<{ | ||
| packageName: string | ||
|
|
@@ -14,10 +15,16 @@ const emit = defineEmits<{ | |
| addNoDep: [] | ||
| }>() | ||
|
|
||
| const docUrl = computed(() => { | ||
| if (props.replacement.type !== 'documented' || !props.replacement.docPath) return null | ||
| return `https://e18e.dev/docs/replacements/${props.replacement.docPath}.html` | ||
| const docUrl = computed(() => resolveDocUrl(props.replacement.url)) | ||
|
|
||
| const nodeVersion = computed(() => { | ||
| const nodeEngine = props.replacement.engines?.find(e => e.engine === 'nodejs') | ||
| return nodeEngine?.minVersion || null | ||
| }) | ||
|
|
||
| const replacementDescription = useMarkdown(() => ({ | ||
| text: (props.replacement as { description?: string }).description ?? '', | ||
| })) | ||
| </script> | ||
|
|
||
| <template> | ||
|
|
@@ -28,6 +35,7 @@ const docUrl = computed(() => { | |
| ? 'bg-amber-500/10 border border-amber-600/30 text-amber-800 dark:text-amber-400' | ||
| : 'bg-blue-500/10 border border-blue-600/30 text-blue-700 dark:text-blue-400' | ||
| " | ||
| data-testid="replacement-suggestion-card" | ||
| > | ||
| <span | ||
| class="w-4 h-4 flex-shrink-0 mt-0.5" | ||
|
|
@@ -36,33 +44,65 @@ const docUrl = computed(() => { | |
| <div class="min-w-0 flex-1"> | ||
| <p class="font-medium">{{ packageName }}: {{ $t('package.replacement.title') }}</p> | ||
| <p class="text-xs mt-0.5 opacity-80"> | ||
| <template v-if="replacement.type === 'native'"> | ||
| {{ | ||
| $t('package.replacement.native', { | ||
| replacement: replacement.replacement, | ||
| nodeVersion: replacement.nodeVersion, | ||
| }) | ||
| }} | ||
| </template> | ||
| <i18n-t | ||
| v-if="nodeVersion && replacement.type === 'native'" | ||
| keypath="package.replacement.native" | ||
| scope="global" | ||
| > | ||
| <template #replacement> | ||
| <span v-if="replacementDescription" v-html="replacementDescription" /> | ||
| <span v-else | ||
| ><code>{{ replacement.id }}</code></span | ||
| > | ||
| </template> | ||
| <template #nodeVersion> | ||
| {{ nodeVersion }} | ||
| </template> | ||
| </i18n-t> | ||
| <i18n-t | ||
| v-else-if="replacement.type === 'native'" | ||
| keypath="package.replacement.native_no_version" | ||
| scope="global" | ||
| > | ||
| <template #replacement> | ||
| <span v-if="replacementDescription" v-html="replacementDescription" /> | ||
| <span v-else | ||
| ><code>{{ replacement.id }}</code></span | ||
| > | ||
| </template> | ||
| </i18n-t> | ||
| <template v-else-if="replacement.type === 'simple'"> | ||
| {{ | ||
| $t('package.replacement.simple', { | ||
| replacement: replacement.replacement, | ||
| community: $t('package.replacement.community'), | ||
| }) | ||
| }} | ||
| <i18n-t keypath="package.replacement.simple"> | ||
| <template #replacement><span v-html="replacementDescription" /></template> | ||
| <template #community>{{ $t('package.replacement.community') }}</template> | ||
| </i18n-t> | ||
| </template> | ||
| <template v-else-if="replacement.type === 'documented'"> | ||
| {{ | ||
| $t('package.replacement.documented', { | ||
| community: $t('package.replacement.community'), | ||
| }) | ||
| }} | ||
| <i18n-t | ||
| v-else-if="replacement.type === 'documented'" | ||
| keypath="package.replacement.documented" | ||
| scope="global" | ||
| > | ||
| <template #replacement> | ||
| <code>{{ replacement.replacementModule }}</code> | ||
| </template> | ||
| <template #community> | ||
| <a | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should use NuxtLink but what do you think ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It also works for external links as far as I know https://nuxt.com/docs/4.x/api/components/nuxt-link#external-routing |
||
| href="https://e18e.dev/docs/replacements/" | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| class="inline-flex items-center gap-1 ms-1 underline underline-offset-4 decoration-amber-600/60 dark:decoration-amber-400/50 hover:decoration-fg transition-colors" | ||
| > | ||
| {{ $t('package.replacement.community') }} | ||
| <span class="i-lucide:external-link w-3 h-3" aria-hidden="true" /> | ||
| </a> | ||
| </template> | ||
| </i18n-t> | ||
| <template v-else-if="replacement.type === 'removal'"> | ||
| <span v-html="replacementDescription" /> | ||
| </template> | ||
| </p> | ||
| </div> | ||
|
|
||
| <!-- No dependency action button --> | ||
| <ButtonBase | ||
| v-if="variant === 'nodep' && showAction !== false" | ||
| size="sm" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,67 +1,69 @@ | ||
| <script setup lang="ts"> | ||
| import type { ModuleReplacement } from 'module-replacements' | ||
| import type { ModuleReplacement, ModuleReplacementMapping } from 'module-replacements' | ||
| import { resolveDocUrl } from 'module-replacements' | ||
|
|
||
| const props = defineProps<{ | ||
| mapping: ModuleReplacementMapping | ||
| replacement: ModuleReplacement | ||
| }>() | ||
|
|
||
| const mdnUrl = computed(() => { | ||
| if (props.replacement.type !== 'native' || !props.replacement.mdnPath) return null | ||
| return `https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/${props.replacement.mdnPath}` | ||
| }) | ||
| const externalUrl = computed(() => resolveDocUrl(props.mapping.url ?? props.replacement.url)) | ||
|
|
||
| const docPath = computed(() => { | ||
| if (props.replacement.type !== 'documented' || !props.replacement.docPath) return null | ||
| return `https://e18e.dev/docs/replacements/${props.replacement.docPath}.html` | ||
| const nodeVersion = computed(() => { | ||
| const nodeEngine = props.replacement.engines?.find(e => e.engine === 'nodejs') | ||
43081j marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return nodeEngine?.minVersion || null | ||
gameroman marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| }) | ||
|
|
||
| const replacementDescription = useMarkdown(() => ({ | ||
| text: (props.replacement as { description?: string }).description ?? '', | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should probably just have a shared function in this repo that computes the description: function getReplacementDescription(replacement) {
switch (replacement.type) {
case 'documented':
return undefined;
default:
return replacement.description;
}
}
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should that be in |
||
| })) | ||
| </script> | ||
|
|
||
| <template> | ||
| <div | ||
| class="border border-amber-600/40 bg-amber-500/10 rounded-lg px-3 py-2 text-base text-amber-800 dark:text-amber-400" | ||
| data-testid="replacement-card" | ||
| > | ||
| <h2 class="font-medium mb-1 flex items-center gap-2"> | ||
| <span class="i-lucide:lightbulb w-4 h-4" aria-hidden="true" /> | ||
| {{ $t('package.replacement.title') }} | ||
| </h2> | ||
| <p class="text-sm m-0"> | ||
| <p> | ||
| <i18n-t | ||
| v-if="replacement.type === 'native'" | ||
| v-if="nodeVersion && replacement.type === 'native'" | ||
| keypath="package.replacement.native" | ||
| scope="global" | ||
| > | ||
| <template #replacement> | ||
| {{ replacement.replacement }} | ||
| <span v-if="replacementDescription" v-html="replacementDescription" /> | ||
| <span v-else | ||
| ><code>{{ replacement.id }}</code></span | ||
| > | ||
| </template> | ||
| <template #nodeVersion> | ||
| {{ replacement.nodeVersion }} | ||
| {{ nodeVersion }} | ||
| </template> | ||
| </i18n-t> | ||
| <i18n-t | ||
| v-else-if="replacement.type === 'simple'" | ||
| keypath="package.replacement.simple" | ||
| v-else-if="replacement.type === 'native'" | ||
| keypath="package.replacement.native_no_version" | ||
| scope="global" | ||
| > | ||
| <template #community> | ||
| <a | ||
| href="https://e18e.dev/docs/replacements/" | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| class="inline-flex items-center gap-1 ms-1 underline underline-offset-4 decoration-amber-600/60 dark:decoration-amber-400/50 hover:decoration-fg transition-colors" | ||
| > | ||
| {{ $t('package.replacement.community') }} | ||
| <span class="i-lucide:external-link w-3 h-3" aria-hidden="true" /> | ||
| </a> | ||
| </template> | ||
| <template #replacement> | ||
| {{ replacement.replacement }} | ||
| <span v-if="replacementDescription" v-html="replacementDescription" /> | ||
| <span v-else | ||
| ><code>{{ replacement.id }}</code></span | ||
| > | ||
| </template> | ||
| </i18n-t> | ||
| <i18n-t | ||
| v-else-if="replacement.type === 'documented'" | ||
| keypath="package.replacement.documented" | ||
| scope="global" | ||
| > | ||
| <template #replacement> | ||
| <code>{{ replacement.replacementModule }}</code> | ||
| </template> | ||
| <template #community> | ||
| <a | ||
| href="https://e18e.dev/docs/replacements/" | ||
|
|
@@ -74,22 +76,18 @@ const docPath = computed(() => { | |
| </a> | ||
| </template> | ||
| </i18n-t> | ||
| <template v-else-if="replacement.type === 'removal'"> | ||
| <span v-html="replacementDescription" /> | ||
| </template> | ||
| <template v-else-if="replacement.type === 'simple'"> | ||
| <span v-html="replacementDescription" /> | ||
| </template> | ||
| <template v-else> | ||
| {{ $t('package.replacement.none') }} | ||
| </template> | ||
| <a | ||
| v-if="mdnUrl" | ||
| :href="mdnUrl" | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| class="inline-flex items-center gap-1 ms-1 underline underline-offset-4 decoration-amber-600/60 dark:decoration-amber-400/50 hover:decoration-fg transition-colors" | ||
| > | ||
| {{ $t('package.replacement.mdn') }} | ||
| <span class="i-lucide:external-link w-3 h-3" aria-hidden="true" /> | ||
| </a> | ||
| <a | ||
| v-if="docPath" | ||
| :href="docPath" | ||
| v-if="externalUrl" | ||
| :href="externalUrl" | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| class="inline-flex items-center gap-1 ms-1 underline underline-offset-4 decoration-amber-600/60 dark:decoration-amber-400/50 hover:decoration-fg transition-colors" | ||
|
|
@@ -98,5 +96,11 @@ const docPath = computed(() => { | |
| <span class="i-lucide:external-link w-3 h-3" aria-hidden="true" /> | ||
| </a> | ||
| </p> | ||
| <div v-if="replacement.type === 'simple' && replacement.example"> | ||
| <strong class="block mb-1.5">{{ $t('package.replacement.example') }}</strong> | ||
| <pre | ||
| class="bg-amber-800/10 dark:bg-amber-950/30 p-2 rounded border border-amber-700/20 overflow-x-auto text-xs font-mono leading-relaxed" | ||
| ><code>{{ replacement.example }}</code></pre> | ||
| </div> | ||
| </div> | ||
| </template> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import type { ModuleReplacement } from 'module-replacements' | ||
| import type { ModuleReplacement, ModuleReplacementMapping } from 'module-replacements' | ||
|
|
||
| async function fetchReplacements( | ||
| deps: Record<string, string>, | ||
|
|
@@ -8,7 +8,11 @@ async function fetchReplacements( | |
| const results = await Promise.all( | ||
| names.map(async name => { | ||
| try { | ||
| const replacement = await $fetch<ModuleReplacement | null>(`/api/replacements/${name}`) | ||
| const response = await $fetch<{ | ||
| mapping: ModuleReplacementMapping | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @danielroe iirc you said these types are inferred based on the route. is that true? does this not infer them because of the interpolation? |
||
| replacement: ModuleReplacement | ||
| } | null>(`/api/replacements/${name}`) | ||
| const replacement = response?.replacement ?? null | ||
| return { name, replacement } | ||
| } catch { | ||
| return { name, replacement: null } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import type { ModuleReplacement } from 'module-replacements' | ||
| import type { ModuleReplacement, ModuleReplacementMapping } from 'module-replacements' | ||
|
|
||
| export interface ReplacementSuggestion { | ||
| forPackage: string | ||
|
|
@@ -41,8 +41,11 @@ export function useCompareReplacements(packageNames: MaybeRefOrGetter<string[]>) | |
| const results = await Promise.all( | ||
| namesToCheck.map(async name => { | ||
| try { | ||
| const replacement = await $fetch<ModuleReplacement | null>(`/api/replacements/${name}`) | ||
| return { name, replacement } | ||
| const result = await $fetch<{ | ||
| mapping: ModuleReplacementMapping | ||
| replacement: ModuleReplacement | ||
| } | null>(`/api/replacements/${name}`) | ||
| return { name, replacement: result?.replacement ?? null } | ||
| } catch { | ||
| return { name, replacement: null } | ||
| } | ||
|
|
@@ -99,9 +102,9 @@ export function useCompareReplacements(packageNames: MaybeRefOrGetter<string[]>) | |
| ) | ||
|
|
||
| return { | ||
| replacements: readonly(replacements), | ||
| noDepSuggestions: readonly(noDepSuggestions), | ||
| infoSuggestions: readonly(infoSuggestions), | ||
| replacements, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are these still readonly? was that dropped on purpose? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. as far as I can see seems by mistake, |
||
| noDepSuggestions, | ||
| infoSuggestions, | ||
| loading: readonly(loading), | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,7 @@ | ||
| import type { ModuleReplacement } from 'module-replacements' | ||
| import type { ModuleReplacement, ModuleReplacementMapping } from 'module-replacements' | ||
|
|
||
| export function useModuleReplacement(packageName: MaybeRefOrGetter<string>) { | ||
| return useLazyFetch<ModuleReplacement | null>(() => `/api/replacements/${toValue(packageName)}`) | ||
| return useLazyFetch<{ mapping: ModuleReplacementMapping; replacement: ModuleReplacement } | null>( | ||
| () => `/api/replacements/${toValue(packageName)}`, | ||
| ) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -884,7 +884,11 @@ const showSkeleton = shallowRef(false) | |
|
|
||
| <div class="space-y-6" :class="$style.areaVulns"> | ||
| <!-- Bad package warning --> | ||
| <PackageReplacement v-if="moduleReplacement" :replacement="moduleReplacement" /> | ||
| <PackageReplacement | ||
| v-if="moduleReplacement" | ||
| :mapping="moduleReplacement.mapping" | ||
| :replacement="moduleReplacement.replacement" | ||
| /> | ||
|
Comment on lines
+887
to
+891
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe we should just call
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That makes sense, yes
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. actually I'm not sure, since it's rendered conditionally based on if
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we should There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we always have to options parent fetchs and decides if renders or not or we put that logic inside the component and inside we have a v-if to not render html etc Maybe you can even have a component packageReplacementIntegration that is responsible to fetch the replacement and the rendering the visual part |
||
| <!-- Size / dependency increase notice --> | ||
| <PackageSizeIncrease v-if="sizeDiff" :diff="sizeDiff" /> | ||
| <!-- Vulnerability scan --> | ||
|
|
||
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.
What do you think of using a type guard instead of relying in casting ?
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.
descriptionis eitherundefinedor astringso it can't be anything elseThere 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 he means this which is safer than casting:
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.
Yeah exactly trying to make the code more typesafe to refactors etc