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.

Sign up for GitHub

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

Jump to bottom

Prevent global namespace pollution in Markdown preview #147936

Closed
yhatt opened this issue Apr 22, 2022 · 4 comments · Fixed by #148503
Closed

Prevent global namespace pollution in Markdown preview #147936

yhatt opened this issue Apr 22, 2022 · 4 comments · Fixed by #148503
Assignees
@mjbvz
Labels
feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities insiders-released Patch has been released in VS Code Insiders markdown Markdown support issues verification-needed Verification of issue is requested verified Verification succeeded
Milestone
May 2022

Comments

@yhatt
Copy link
Contributor

yhatt commented Apr 22, 2022

A script for Markdown preview from my extension accidentally met collision with a minimized global variable provided by VS Code. It brought a broken Markdown preview that is difficult to debug a problem.
marp-team/marp-vscode#345 (comment)

I think extension authors might want to take care with global namespace pollution, but applying IIFE into VS Code's Markdown preview script also would be better to save extensions from accidental breaking.
@mjbvz mjbvz added help wanted Issues identified as good community contribution opportunities feature-request Request for new features or functionality labels Apr 30, 2022
@mjbvz
Copy link
Collaborator

mjbvz commented Apr 30, 2022

Please submit a PR for this

@mjbvz mjbvz added the markdown Markdown support issues label Apr 30, 2022
yhatt added a commit to yhatt/vscode that referenced this issue May 1, 2022
@yhatt
Fixes microsoft#147936
dc6e6df 
By changing output esbuild setting for Markdown preview scripts to iife,
prevents global namespace pollution and reduces possibility of breaking
the preview by contributions from other extensions.
@yhatt yhatt mentioned this issue May 1, 2022
Merged
@yhatt
Copy link
Contributor Author

yhatt commented May 1, 2022

OK, I've submitted PR #148503.

mjbvz pushed a commit that referenced this issue May 2, 2022
@yhatt
Fixes #147936 (#148503)
82ad6af 
By changing output esbuild setting for Markdown preview scripts to iife,
prevents global namespace pollution and reduces possibility of breaking
the preview by contributions from other extensions.
sourcegraph-bot pushed a commit to sgtest/megarepo that referenced this issue May 2, 2022
vscode: Fixes microsoft/vscode#147936 (#148503)
7c13ec5 
Commit: 82ad6afd364942f5313e21618c2d05e824fbcce5
@mjbvz mjbvz modified the milestones: Backlog Candidates, May 2022 May 2, 2022
@mjbvz mjbvz added the author-verification-requested Issues potentially verifiable by issue author label May 28, 2022
@VSCodeTriageBot
Copy link
Collaborator

This bug has been fixed in the latest release of VS Code Insiders!

@yhatt, you can help us out by commenting /verified if things are now working as expected.

If things still don't seem right, please ensure you're on version 6428d0f of Insiders (today's or later - you can use Help: About in the command palette to check), and leave a comment letting us know what isn't working as expected.

Happy Coding!

@yhatt
Copy link
Contributor Author

yhatt commented May 28, 2022

/verified

@VSCodeTriageBot VSCodeTriageBot added verified Verification succeeded and removed author-verification-requested Issues potentially verifiable by issue author labels May 28, 2022
@joaomoreno joaomoreno added the verification-needed Verification of issue is requested label Jun 1, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Jun 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@mjbvz mjbvz

Labels
feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities insiders-released Patch has been released in VS Code Insiders markdown Markdown support issues verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Milestone
May 2022
Development

Successfully merging a pull request may close this issue.

Fixes #147936 yhatt/vscode
5 participants
@joaomoreno @yhatt @mjbvz @VSCodeTriageBot and others