Skip to content
This repository has been archived by the owner on Apr 6, 2023. It is now read-only.

feat(nuxt): add fetchpriority attribute and literal typings for meta components #6251

Merged
merged 18 commits into from
Aug 2, 2022
Merged

feat(nuxt): add fetchpriority attribute and literal typings for meta components #6251

merged 18 commits into from
Aug 2, 2022

Conversation

sanjaiyan-dev
Copy link
Contributor

Added fetchpriority attr in script and link component.

@netlify
Copy link

netlify bot commented Jul 29, 2022 •
edited

✅ Deploy Preview for nuxt3-docs canceled.

Name Link
🔨 Latest commit 9ca4154
🔍 Latest deploy log https://app.netlify.com/sites/nuxt3-docs/deploys/62e91246fca631000893a819

@sanjaiyan-dev sanjaiyan-dev changed the title Sanjaiyan-fetchpiritory-attr Adding fetchpriority attr Jul 29, 2022
@danielroe danielroe changed the title Adding fetchpriority attr feat(nuxt): add fetchpriority attr to <Script> and <Link> Jul 29, 2022
@DamianGlowala
Copy link
Member

Shall we narrow down the accepted values for the fetchpriority attribute, as per https://wicg.github.io/priority-hints/#definitions?

@DamianGlowala DamianGlowala mentioned this pull request Jul 30, 2022
Closed
7 tasks
DamianGlowala
DamianGlowala suggested changes Jul 30, 2022
View reviewed changes
packages/nuxt/src/head/runtime/components.ts Outdated Show resolved Hide resolved
packages/nuxt/src/head/runtime/components.ts Outdated Show resolved Hide resolved
@sanjaiyan-dev
Copy link
Contributor Author

Shall we narrow down the accepted values for the fetchpriority attribute, as per https://wicg.github.io/priority-hints/#definitions?

💪🏽

@sanjaiyan-dev
Copy link
Contributor Author

Sorry, I have copied the code from #6262 for adding typings for reference-policy attribute 🙇🏽

@sanjaiyan-dev sanjaiyan-dev changed the title feat(nuxt): add fetchpriority attr to <Script> and <Link> feat(nuxt): add fetchpriority attr to <Script> and <Link> and minor improvements in types :) Jul 30, 2022
@sanjaiyan-dev sanjaiyan-dev changed the title feat(nuxt): add fetchpriority attr to <Script> and <Link> and minor improvements in types :) feat(nuxt): Add fetchpriority attr to <Script> and <Link> and minor improvements in types :) Jul 30, 2022
@DamianGlowala
Copy link
Member

Great enhancement! I have closed #6262 in favor of your changes. There are two remarks that I've got:

@sanjaiyan-dev
Copy link
Contributor Author

sanjaiyan-dev commented Jul 31, 2022 •
edited

Great enhancement! I have closed #6262 in favor of your changes. There are two remarks that I've got:

  1. Boolean type is equivalent to crossorigin="" which is shorthand way of writing crossorigin="anonymous" so this makes typing code easier ,
<img crossorigin />

is easier than

<img crossorigin="" />

And even browser parse the html with crossorigin attr empty string as <link crossorigin /> (not sure)
I am not 100% sure about it :
so, do we need to add boolean type too ?

@sanjaiyan-dev
Copy link
Contributor Author

Great enhancement! I have closed #6262 in favor of your changes. There are two remarks that I've got:

  1. Boolean type is equivalent to crossorigin="" which is shorthand way of writing crossorigin="anonymous" so this makes typing code easier ,
<img crossorigin />

is easier than

<img crossorigin="" />

And even browser parse the html with crossorigin attr empty string as <link crossorigin /> (not sure) I am not 100% sure about it : so, do we need to add boolean type too ?

I am sorry if I am wrong :(

@DamianGlowala
Copy link
Member

All I am thinking is whether the following would be preferable:

crossorigin: String as PropType<CrossOrigin>

Let's wait for others to express their thoughts, not sure which way to go :)

@sanjaiyan-dev
Copy link
Contributor Author

All I am thinking is whether the following would be preferable:

crossorigin: String as PropType<CrossOrigin>

Let's wait for others to express their thoughts, not sure which way to go :)

Ok 🙌

@sanjaiyan-dev
Copy link
Contributor Author 

type LinkRelationship =
  | "alternate"
  | "author"
  | "canonical"
  | "dns-prefetch"
  | "help"
  | "icon"
  | "license"
  | "manifest"
  | "me"
  | "modulepreload"
  | "next"
  | "pingback"
  | "preconnect"
  | "prefetch"
  | "preload"
  | "prerender"
  | "prev"
  | "search"
  | "stylesheet"
  | String;

Added types for <link /> tag with String constructor to accept any future value while preserving intellisense 😄

danielroe
danielroe suggested changes Aug 1, 2022
View reviewed changes
Copy link
Member

@danielroe danielroe left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love the direction of this PR. ❤️ Would you revert the various style/lint changes that have been made by your IDE?

@sanjaiyan-dev
Copy link
Contributor Author

Love the direction of this PR. ❤️ Would you revert the various style/lint changes that have been made by your IDE?

Yeah sure 🙌

@sanjaiyan-dev
Copy link
Contributor Author

Made suggested change (not sure) 😄

@pi0 pi0 requested a review from danielroe August 2, 2022 11:44
@pi0 pi0 changed the title feat(nuxt): Add fetchpriority attr to <Script> and <Link> and minor improvements in types :) feat(nuxt): add fetchpriority attribute to the <Script> and <Link> components Aug 2, 2022
danielroe
danielroe approved these changes Aug 2, 2022
View reviewed changes
Copy link
Member

@danielroe danielroe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice enhancement - thanks! ❤️

@danielroe danielroe changed the title feat(nuxt): add fetchpriority attribute to the <Script> and <Link> components feat(nuxt): add fetchpriority attribute and literal typings for meta components Aug 2, 2022
pi0
pi0 approved these changes Aug 2, 2022
View reviewed changes
Copy link
Member

@pi0 pi0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@pi0 pi0 merged commit 94d0c08 into nuxt:main Aug 2, 2022
@sanjaiyan-dev
Copy link
Contributor Author

Thanks!

My pleasure 💪🙌

@pi0 pi0 mentioned this pull request Aug 5, 2022
Merged
This was referenced Aug 5, 2022
Merged
Closed
@danielroe danielroe added the 3.x label Jan 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@danielroe danielroe danielroe approved these changes

@DamianGlowala DamianGlowala DamianGlowala requested changes

@pi0 pi0 pi0 approved these changes

Assignees
No one assigned
Labels
3.x
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@sanjaiyan-dev @DamianGlowala @pi0 @danielroe