Skip to content

Encode and decode copy-to-clipboard functionality using base64#372

Merged
mishig25 merged 1 commit into
huggingface:mainfrom
xenova:main
May 15, 2023
Merged

Encode and decode copy-to-clipboard functionality using base64#372
mishig25 merged 1 commit into
huggingface:mainfrom
xenova:main

Conversation

@xenova

@xenova xenova commented May 12, 2023

Copy link
Copy Markdown
Contributor

Fixes issue where the code attribute for copy-to-clipboard functionality is interpreted as valid Svelte code, which leads to SSR errors.

@xenova

xenova commented May 12, 2023

Copy link
Copy Markdown
Contributor Author

The failed tests seem to be related to a recent update of timm, and shouldn't have anything to do with this PR.

@coyotte508

Copy link
Copy Markdown
Member

Can you give an example of markdown that fails with current code?

@xenova

xenova commented May 13, 2023

Copy link
Copy Markdown
Contributor Author

Can you give an example of markdown that fails with current code?

Sure thing - sorry, I had mentioned to @mishig25 privately.

# This is test\n\n
```jsx
let worker = new Worker(new URL('./worker.js', import.meta.url));

@coyotte508

coyotte508 commented May 13, 2023

Copy link
Copy Markdown
Member

if you add a newline between #this is test and the code sample, does the problem still happen?

Edit ok, tested and it doesn't work:
image

Trying with your branch

Edit 2: Same error. PR: https://github.com/huggingface/huggingface.js/pull/192/files

@xenova

xenova commented May 13, 2023

Copy link
Copy Markdown
Contributor Author

That's odd; it works locally.

Here's the original version rendered correctly:

image

I did notice that, when testing locally, it sometimes uses a cached version of the doc builder.

Edit: I see it's actually producing a different error, and it's trying to evaluate the worker instantiation.

@coyotte508

Copy link
Copy Markdown
Member

Ok after a few tries your fix worked! :) (github action cache...)

Approving, leaving it up to @mishig25 to take at second look & merge next week

@coyotte508 coyotte508 requested a review from mishig25 May 13, 2023 19:52
mishig25 added a commit to huggingface/transformers that referenced this pull request May 15, 2023

@mishig25 mishig25 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.

lgtm!
I've tested the build on transformers docs & everything is working as expected huggingface/transformers#23361

Therefore, merging the PR 🚀

(test is failing for unrelated reason to this PR)

@mishig25 mishig25 merged commit c59346e into huggingface:main May 15, 2023
Sign up for free to 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