fix DoNotTranslation deserialization #16725
Merged
Conversation
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
|
Hi @tabathadelane 👋 Thanks for your pull request! Your PR is in a queue, and a writer will take a look soon. We generally publish small edits within one business day, and larger edits within three days. We will automatically generate a preview of your request, and will comment with a link when the preview is ready (usually 10 to 20 minutes). If you add any more commits, you can comment |
✅ Deploy Preview for docs-website-netlify ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
jeff-colucci
added
the
eng
`mdxJsxTextElement` is a node type elsewhere, but it doesn't work here
deserialization-helpers.js was using an older version of unified and related packages, which was causing it not to recognize `mdxJsx*` node types. upgrading in that file would've been difficult, because it would've had to change to an ESM file, as would handlers.js, and many more files in the dependency chain. this is a decent workaround — just define the attribute processor in deserialize-html.mjs, then pass it as an argument through to the deserialization-helper functions to get tests working, i had to add `TRANSLATION_TYPE=human` to the test command, i wasn't able to get mocking the configuration module working
sunnyzanchi
force-pushed
the
tabatha/dnt-new-lines-2
branch
from
e6b40d1 to
afe40e6
Compare
sunnyzanchi
changed the title
sunnyzanchi
force-pushed
the
tabatha/dnt-new-lines-2
branch
from
afe40e6 to
effd4d0
Compare
✅ Deploy Preview for docs-website-kr ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
❌ Deploy Preview for docs-website-es failed. Why did it fail? →
|
❌ Deploy Preview for docs-website-pt failed. Why did it fail? →
|
sunnyzanchi
requested review from
a team,
sunnyzanchi and
LizBaker
and removed request for
a team
clarkmcadoo
reviewed
Apr 24, 2024
clarkmcadoo
previously approved these changes
Apr 24, 2024
…/docs-website into tabatha/dnt-new-lines-2
this ensures we keep brackets around MDX expressions
…to tabatha/dnt-new-lines-2
|
Hi @tabathadelane , is this PR still something we're actively working on? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
this PR does two main things:
add upgraded versions of unified and related packages
you can see these in code asunified11orremark-parse10, etc. it would be a huge undertaking to upgrade everywhere we use these packages, so this numbering lets us use the upgraded versions only in specific places. upgrading these packages in our deserialization code fixes the newline issue with DNT tags. there were some other minor changes necessary to get deserialization working with the upgraded packagesadd
rather than desperately fight Jest to somehow get CJS and ESM tests working, this PR addsuvuand Sinon for ESM testsuvuspecifically for running ESM tests. ESM tests are necessary as newer unified packages only have ESM support (no CJS), so our code has to be ESM as well. our test command now runs both Jest and uvu. one small note is thatuvudoes not support snapshot testing. i wrote a rudimentary module to accomplish this. i added Sinon for easier function mocking, asjest.fncan't be used outside of Jestserialization code is unchanged with this PR. i tried to keep the scope as tight as possible, so this only updates deserialization code and other files that needed to be updated to ESM out of necessity