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

Mass updates and related code migrations/type fixes #50

Merged
merged 8 commits into from
Oct 22, 2024

Conversation

rosskevin
Copy link

@rosskevin rosskevin commented Sep 6, 2024

I tried to keep all intents the same. I note there is no eslint config.

  • update all libraries
  • migrate away from deprecated vscode to vscode-test and @types/vscode
  • update type errors
  • remove some deprecated code to fix warnings

Scripts:

  • add build script (no watch)
  • move compile to build:watch
  • add package scripts to use vsce in a way that allows for modern yarn use
  • add convenience package:install for local dev use
  • add deploy scripts

#49

I'm not familiar with writing/testing vscode extensions, so this should be reviewed before merging.

Related:

@iRyusa
Copy link
Member

iRyusa commented Sep 6, 2024

👋 Thanks for this, can you check to remove that .yarn stuff you've commited ?

yarn v2+ not supported by vsce
microsoft/vscode-vsce#517

draw inspiration from abcradabra (linked in above issue)
nicoespeon/abracadabra@10e0c3d
@rosskevin
Copy link
Author

Hey @iRyusa - I just pushed more changes, let me update the OP...one moment

@rosskevin
Copy link
Author

rosskevin commented Sep 6, 2024

@iRyusa I think this is good to review now. I bumped the version just for safety, you can revert that or change as needed.

In our open repos, we always ship the .yarn stuff (no cache) so everyone is always on the same page. I believe the script additions makes this feasible.

Of course you are free to change that, but I think the overall package now is buildable, packageable and deployable as long as long as someone is using a yarn 2+.

There are different ways to accomplish the same thing though...I'm just wanting to see this easily contributed to by many.

@rosskevin
Copy link
Author

rosskevin commented Sep 6, 2024

@iRyusa currently if you locally package:install the vsix - the old (deployed) plugin disappears and the new does not appear at all - so not all is quite well.

@rosskevin
Copy link
Author

Wondering if this needs to further similar to abcracadabra and use esbuild https://code.visualstudio.com/api/working-with-extensions/bundling-extension

?

@rosskevin
Copy link
Author

.yarn is out, the ignores remain.

@rosskevin
Copy link
Author

I'm going to leave this branch/PR as-is but branch from here and try the recommended esbuild path.

@rosskevin
Copy link
Author

rosskevin commented Sep 6, 2024

NOTE: the esbuild path is raising good errors - showing things that will break at runtime (likely the problem) such as:

▲ [WARNING] Calling "mjml2html" will crash at run-time because it's an import namespace object, not a function [call-import-namespace]

    src/helper.ts:66:15:
      66 │         return mjml2html(mjml, {

so I'll keep pursuing that on that newer branch.

@rosskevin
Copy link
Author

@iRyusa any chance you can try to run this and let me know where I should look to fix it?

@iRyusa
Copy link
Member

iRyusa commented Oct 22, 2024

I'm trying to get this merge and deployed today + some cleaning on the repo (removing phantomJS, MJML5,...)

@iRyusa iRyusa merged commit 21b5f23 into mjmlio:master Oct 22, 2024
@iRyusa
Copy link
Member

iRyusa commented Oct 22, 2024

@rosskevin Massive thanks for this I had to remove some of old deps that cause some issue (ESModule 😬)

This is published as 2.0.0 with MJML5 branch on it, and some older command are removed.
Again, sorry for the late merge and reply

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.

2 participants