Skip to content

fix(transformers): opt-in to correct extensions spelling - #1213

Closed
harlan-zw wants to merge 4 commits into
nuxt:mainfrom
harlan-zw:fix/use-soft-breaking-change-extensions
Closed

fix(transformers): opt-in to correct extensions spelling#1213
harlan-zw wants to merge 4 commits into
nuxt:mainfrom
harlan-zw:fix/use-soft-breaking-change-extensions

Conversation

@harlan-zw

@harlan-zwharlan-zw commented Jun 5, 2022

Copy link
Copy Markdown
Contributor

🔗 Linked issue

Related #1204 (comment)

❓ Type of change

  • 📖 Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • 👌 Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

This is an alternative solution to the fix for mispelled extensions, this PR reverts the breaking change commit of #1204 (comment). We prefer to use a soft breaking change to avoid having to bump majors.

I've also included a test for a custom transformer here using the deprecated key.

📝 Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@netlify

netlifyBot commented Jun 5, 2022

Copy link
Copy Markdown

‼️ Deploy request for nuxt-content rejected.

NameLink
🔨 Latest commit2733907

@harlan-zw

Copy link
Copy Markdown
ContributorAuthor

Hm I wanted to revert the other commit but have done something wrong 😬

@harlan-zw

Copy link
Copy Markdown
ContributorAuthor

Okay should be good

@Tahul

Tahul commented Jun 7, 2022

Copy link
Copy Markdown
Contributor

That you a lot @harlan-zw 🙂

Should've renamed the original PR, this is my bad!

The new test is a great addition as well!

@Tahul

Tahul commented Jun 7, 2022

Copy link
Copy Markdown
Contributor

I would just like to double-check with @atinux and @farnabaz about keeping that extentions naming in the codebase with @deprecated.

I think no one (or very few people) yet used these features, avoiding preserving that kind of dead code could be great... even if I understand the concerns about semantic versioning.

WDYT @farnabaz@atinux ?

@farnabaz

Copy link
Copy Markdown
Member

I agree that no one yet used this feature, also we do not have good documentation about parsers and transformers.

Transformers are mainly used internally, We did not expose types and a helper to create custom transformers yet. So I see no harm in dropping misspelled key.

It will be good if we create/expose util to define transformers. like defineContentTransformer. This way we ensure users have full ts support and better DX

@TahulTahul left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you please remove the @deprecated keys and support for previous implementation?

I would like to preserve the test-case there, but not the support for older implementations (point about this in discussion with @farnabaz above).

@harlan-zw

harlan-zw commented Jun 8, 2022

Copy link
Copy Markdown
ContributorAuthor

I'll make a seperate pr for the test case if not needed 👍

@harlan-zw

Copy link
Copy Markdown
ContributorAuthor

I agree that no one yet used this feature, also we do not have good documentation about parsers and transformers.

Transformers are mainly used internally, We did not expose types and a helper to create custom transformers yet. So I see no harm in dropping misspelled key.

It will be good if we create/expose util to define transformers. like defineContentTransformer. This way we ensure users have full ts support and better DX

Great idea

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@harlan-zw@Tahul@farnabaz