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

fix(editor/vscode): set minimum supported ver. to ^1.93.0 #8182

Merged
merged 4 commits into from
Dec 30, 2024

Conversation

pashokitsme
Copy link
Contributor

Hi,

I'm using Cursor which is based on VSCode 1.93 at the moment of this PR. And i've faced with an issue when i can't install latest version of oxc vscode plugin - by default plugin marketplace offers me v0.0.1 and via Install Specific Version i can install only v0.10.3 while latest is v0.15.3. No idea when Cursor will update its version.
There's no issue with VSCode, only Cursor.

After downgrading minimum version, Cursor allowed me to install plugin that i compiled by myself, everything seem to be working.

Not sure about how is usually contributions made here so correct me if i miss something

Copy link

graphite-app bot commented Dec 29, 2024

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

@github-actions github-actions bot added A-editor Area - Editor and Language Server C-bug Category - Bug labels Dec 29, 2024
@Sysix
Copy link
Collaborator

Sysix commented Dec 29, 2024

@nrayburn-tech 1.93 is released in August 2024.
Only supporting the latest version does not sounds right. What do you think?
In my company, we do not get always the lastest update for vscode immediately.

@pashokitsme thanks for the fix and the details why this is needed.
Can you tell renovate to ignore this npm package?

I searched the documentation for you:
https://docs.renovatebot.com/configuration-options/#ignoredeps

@nrayburn-tech
Copy link
Collaborator

Unless there’s a reason for the Oxc plugin to target a more recent version, I wouldn’t update it as a general rule. That being said, to avoid supporting really old versions we could target the current version minus 6 to only support 6 months worth of VS Code releases ( 6 just being an example, we could pick whatever).

So, targeting an older version is probably fine so long as it’s not ancient. I don’t have any concerns with this change.

Something to consider when targeting an older version is you might want to run tests against the old version and the latest version to ensure you are compatible with new releases as they come out.

@camc314
Copy link
Contributor

camc314 commented Dec 29, 2024

we could target the current version minus 6 to only support 6 months worth of VS Code releases ( 6 just being an example, we could pick whatever).

👍 this seems reasonable

@pashokitsme
Copy link
Contributor Author

target the current version minus 6 to only support 6 months

Don't really know how to configure renovate like this. Something like?:

"packageRules": [
  {
    "matchPackagePatterns": ["@types/vscode"],
    "minimumReleaseAge": "6 months"
  }
]

And will renovate update not only devDependencies, but version in engines too? Feels like it won't

@camc314
Copy link
Contributor

camc314 commented Dec 29, 2024

Based on the docs that looks correct.

As for keeping engines in sync

"packageRules": [
  {
    "matchPackagePatterns": ["@types/vscode"],
    "minimumReleaseAge": "6 months",
    "postUpgradeTasks": {
        "commands": [
          "jq '.engines.vscode = \"^\" + (.dependencies.vscode | ltrimstr(\"^\"))' package.json > tmp.json && mv tmp.json package.json"
        ],
        "fileFilters": ["package.json"]
    }
  }
]

didn't test this, but chat gpt suggests this?

But actually, perhaps we want to keep @types/vscode up to date, and simply specify an older supported version in engines?

@Sysix
Copy link
Collaborator

Sysix commented Dec 29, 2024

But actually, perhaps we want to keep @types/vscode up to date, and simply specify an older supported version in engines?

The build will fail because it detects different versions.

Don't really know how to configure renovate like this.

I do not think you need to fix this in this PR.

@nrayburn-tech
Copy link
Collaborator

I would suggest downgrade the version here. Configure renovate to ignore it for now.

Once somebody can test how to get it working correctly (update to the 6 month old version and keep engines in sync) that can be a separate PR. From the docs, I do think something like @camc314 shared should work but I don’t have experience with Renovate.

This isn’t really a critical package to keep up to date for vulnerabilities or things of that nature, so I think it’s low risk to ignore for now.

@pashokitsme
Copy link
Contributor Author

Let's merge it then

@camc314
Copy link
Contributor

camc314 commented Dec 29, 2024

lock file needs updating

@pashokitsme
Copy link
Contributor Author

Oops. Didn't noticed. Fixed

@Boshen Boshen merged commit 0794bee into oxc-project:main Dec 30, 2024
18 checks passed
Boshen added a commit that referenced this pull request Dec 30, 2024
## [0.15.4] - 2024-12-30

### Features

- 47cea9a linter: Implement `eslint/no-extra-label` (#8181) (Anson
Heung)
- ef76e28 linter: Implement `eslint/no-multi-assign` (#8158) (Anson
Heung)
- 384858b linter: Implement `jsx-a11y/no-noninteractive-tabindex`
(#8167) (Tyler Earls)
- afc21a6 linter: Implement `eslint/vars-on-top` (#8157) (Yuichiro
Yamashita)
- 65796c4 linter: Implement `eslint/prefer-rest-params` (#8155)
(Yuichiro Yamashita)
- 5234d96 linter: Implement `eslint/no-nested-ternary` (#8150) (Yuichiro
Yamashita)
- 1c5db72 linter: Implement eslint/no-labels (#8131) (Anson Heung)
- 0b04288 linter: Move `import/named` to nursery (#8068) (Boshen)

### Bug Fixes

- 0794bee editor/vscode: Set minimum supported ver. to `^1.93.0` (#8182)
(Pavel Smirnov)
- f3050d4 linter: Exclude svelte files from `no_unused_vars` rule
(#8170) (Yuichiro Yamashita)
- faf7464 linter: Disable rule `react/rules-of-hook` by file extension
(#8168) (Alexander S.)
- 1171e00 linter: Disable `react/rules-of-hooks` for vue and svelte
files (#8165) (Alexander S.)
- 1b9a5ba linter: False positiver in private member expr in
oxc/const-comparison (#8164) (camc314)
- 6bd9ddb linter: False positive in `typescript/ban-tslint-comment`
(#8094) (dalaoshu)
- 10a1fd5 linter: Rule: `no-restricted-imports` support option
`patterns` with `group` key (#8050) (Alexander S.)
- b3f38ae linter: Rule `no-restricted-imports`: support option
`allowImportNames` (#8002) (Alexander S.)
- 340cc90 linter: Rule `no-restricted-imports`: fix option "importNames"
(#7943) (Alexander S.)
- ec2128e linter: Fix line calculation for `eslint/max-lines` in
diagnostics (#7962) (Dmitry Zakharov)
- 79af100 semantic: Reference flags not correctly resolved when after an
export stmt (#8134) (camc314)

### Performance

- d8d2ec6 linter: Run rules which require typescript syntax only when
source type is actually typescript (#8166) (Alexander S.)
- 2736657 semantic: Allocate `UnresolvedReferences` in allocator (#8046)
(Boshen)

### Refactor

- de8246b language_server: Move structs into own file (#8026) (Alexander
S.)
- 774babb linter: Read `exported_bindings_from_star_export` lazily
(#8062) (Boshen)
- 547c102 linter: Use `RwLock<FxHashMap>` instead of `FxDashMap` for
module record data (#8061) (Boshen)
- 952d7e4 linter: Rename `flat.rs` to `config.rs` (#8033) (camc314)
- 50848ed linter: Simplify `ConfigStore` to prep for nested configs
(#8032) (camc314)
- b2a4a78 linter: Remove unused `with_rules` and `set_rule` methods
(#8029) (camc314)
- 6da0b21 oxlint: Remove unused `git.rs` (#7990) (Boshen)
- 58e7777 oxlint: Remove extra if check in `Walkdir` (#7989) (Boshen)
- 02f968d semantic: Change `Bindings` to a plain `FxHashMap` (#8019)
(Boshen)

Co-authored-by: Boshen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-editor Area - Editor and Language Server C-bug Category - Bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants