Skip to content
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

feat: Added encoding arg to "loadNodeContent" method #26174

Conversation

alexandrtovmach
Copy link
Contributor

Description

Added second argument to set custom encoding in loadNodeContent method.

Documentation

https://www.gatsbyjs.org/docs/node-api-helpers/#loadNodeContent
In PR added more details to documentation.

Related Issues

#26173

@alexandrtovmach alexandrtovmach requested a review from a team as a code owner July 31, 2020 19:20
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Jul 31, 2020
@alexandrtovmach alexandrtovmach added the topic: source-plugins Relates to the Gatsby source plugins (e.g. -filesystem) label Jul 31, 2020
@alexandrtovmach alexandrtovmach linked an issue Jul 31, 2020 that may be closed by this pull request
@alexandrtovmach alexandrtovmach marked this pull request as draft July 31, 2020 19:47
@alexandrtovmach
Copy link
Contributor Author

alexandrtovmach commented Jul 31, 2020

Find another thing that's Node.js doesn't support windows-1251 natively:
https://github.com/nodejs/node/blob/a97b5f9c6acd101ec20d9278a840b2cb6ef94ac9/deps/npm/node_modules/sorted-union-stream/node_modules/string_decoder/index.js#L27

Converted PR to draft to think deeper

@alexandrtovmach alexandrtovmach marked this pull request as ready for review July 31, 2020 20:00
@alexandrtovmach
Copy link
Contributor Author

Added iconv-lite to support as much as possible encodings

@pieh
Copy link
Contributor

pieh commented Aug 3, 2020

This adds 2nd param (encoding), but it isn't actually used anywhere? Now it's up to transformers to make use of it? Do you use custom transformer in the usecase you shared in #26173 where you would make use of it?

@vladar vladar added status: needs core review Currently awaiting review from Core team member and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Aug 3, 2020
@alexandrtovmach
Copy link
Contributor Author

Hey,
You're totally right, this is an optional parameter, and shouldn't be a BREAKING CHANGE for anyone. If transformer maintainers need this, they just can pass custom encoding. For my case it was a custom transformer to handle legacy code files, that used windows-1251 encoding.

@alexandrtovmach
Copy link
Contributor Author

@pieh Sorry for lack of activity here for last year))
I resolved conflicts, could you take a look please?

@LekoArts
Copy link
Contributor

Hey! Thanks so much for opening this pull request!

Sadly this PR got stale and didn't have any activity for some time. We're trying to do better with PR reviews! To get a better overview of all actionable PRs we're going through all open PRs and triage them. Since we won't be able to do everything and adding new features always means added maintenance burden, we have to be more picky about what's beneficial for the average user and the project itself longterm.

Having said all this, we dropped the ball on this PR and we're sorry about not responding to it earlier. As said above, we want to do better in the future but here we should have communicated earlier what will happen with the PR.

Unfortunately we won't merge this PR at the moment because there doesn't seem to be a need for it (since no one else requested it).

If you still think that this should be added, please open a feature request in our discussions. Thanks!

We absolutely want to have you as a contributor and are sorry for any inconveniences we caused with replying too late to this PR.

Thanks for submitting to Gatsby! 💜

@LekoArts LekoArts closed this Jul 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs core review Currently awaiting review from Core team member topic: source-plugins Relates to the Gatsby source plugins (e.g. -filesystem)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pass custom encoding to loadNodeContent method
4 participants