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

Publish on OpenVSX #648

Merged
merged 1 commit into from
Oct 16, 2024
Merged

Publish on OpenVSX #648

merged 1 commit into from
Oct 16, 2024

Conversation

andyw8
Copy link
Contributor

@andyw8 andyw8 commented Oct 15, 2024

Closes #646

@andyw8
Copy link
Contributor Author

andyw8 commented Oct 15, 2024

Looking into this failure:

x % yarn run package      
yarn run v1.22.22
$ vsce package --out vscode-shopify-ruby.vsix
/Users/andyw8/src/github.com/Shopify/vscode-shopify-ruby/node_modules/hosted-git-info/index.js:6
const cache = new LRU({ max: 1000 })
              ^

TypeError: LRU is not a constructor
    at Object.<anonymous> (/Users/andyw8/src/github.com/Shopify/vscode-shopify-ruby/node_modules/hosted-git-info/index.js:6:15)
    at Module._compile (node:internal/modules/cjs/loader:1504:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1588:10)
    at Module.load (node:internal/modules/cjs/loader:1282:32)
    at Module._load (node:internal/modules/cjs/loader:1098:12)
    at TracingChannel.traceSync (node:diagnostics_channel:315:14)
    at wrapModuleLoad (node:internal/modules/cjs/loader:215:24)
    at Module.require (node:internal/modules/cjs/loader:1304:12)
    at require (node:internal/modules/helpers:123:16)
    at Object.<anonymous> (/Users/andyw8/src/github.com/Shopify/vscode-shopify-ruby/node_modules/@vscode/vsce/out/package.js:48:30)

@andyw8
Copy link
Contributor Author

andyw8 commented Oct 15, 2024

^ I solved the above by removing lru-cache from resolutions then regenerating yarn.lock, but that may not be the best way.

@andyw8 andyw8 marked this pull request as ready for review October 15, 2024 17:20
@andyw8 andyw8 requested a review from a team as a code owner October 15, 2024 17:20
@andyw8 andyw8 requested review from st0012 and alexcrocha October 15, 2024 17:20
@@ -27,3 +27,8 @@ jobs:
run: node_modules/.bin/vsce publish --packagePath vscode-shopify-ruby.vsix
env:
VSCE_PAT: ${{ secrets.VSCE_PAT }}

- name: Publish extension on OpenVSX
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copied from ruby-lsp.

dev.yml Outdated
@@ -5,7 +5,7 @@ type: nodejs
up:
- node:
yarn: true
version: 18.15.0
version: 20.9.0
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bumped while troubleshooting. Might not have been necessary, but it now matches Ruby LSP.

@vinistock
Copy link
Member

^ I solved the above by removing lru-cache from resolutions then regenerating yarn.lock, but that may not be the best way.

The resolutions were there to prevent installing a vulnerable version of lru-cache. Is there a way to fix this without removing it?

@andyw8
Copy link
Contributor Author

andyw8 commented Oct 15, 2024

(Confirmed with Vini that the lru-cache entry wasn't for a vulnerability, it was added here)

@andyw8
Copy link
Contributor Author

andyw8 commented Oct 15, 2024

I've added a resolution for hosted-git-info, which avoids the previous issue, and allow for a newer lru-cache. Now fails for a different reason:

% yarn package
yarn run v1.22.22
$ vsce package --out vscode-shopify-ruby.vsix
 INFO  Detected presence of yarn.lock. Using 'yarn' instead of 'npm' (to override this pass '--no-yarn' on the command line).
Executing prepublish script 'yarn run vscode:prepublish'...
$ yarn run esbuild-base --minify
$ esbuild ./src/extension.ts --bundle --outfile=out/extension.js --external:vscode --format=cjs --platform=node --minify

  out/extension.js  1.2kb

 ERROR  GitHost.fromUrl is not a function

@andyw8
Copy link
Contributor Author

andyw8 commented Oct 16, 2024

fromUrl is defined by https://github.com/npm/hosted-git-info, and imported by https://github.com/microsoft/vscode-vsce/blob/main/src/package.ts, so it's strange that it's failing.

Comment on lines +32 to +34
run: |
yarn run package
node_modules/.bin/ovsx publish vscode-shopify-ruby.vsix -p ${{ secrets.OPENVSX_TOKEN }} --yarn

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are only using ovsx to publish our extension, would it make sense to use yarn dlx here instead of adding a dependency?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with that command, but ovsx is only a development dependency. What's the advantage of using dlx?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was checking the ovsx documentation and noticed they use npx in their examples. dlx is the Yarn equivalent of npx. It allows us to run ovsx directly without installing it as a permanent dependency.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. I think it's better to have the explicit dependency to ensure we're running the same versions of ovsx and its dependencies in all environments. Otherwise it could be more difficult when troubleshooting.

@andyw8 andyw8 merged commit 033a4fa into main Oct 16, 2024
4 checks passed
@andyw8 andyw8 deleted the andyw8/open-vsx branch October 16, 2024 16:48
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.

Publish this extension on OpenVSX as well
3 participants