-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat: Add editor SDK support for oxc (oxfmt & oxlint) #7078
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
base: master
Are you sure you want to change the base?
Changes from 6 commits
46e9fd0
697bf98
adbf2a6
0bb61c6
4dc776c
b37c7c1
64ff057
fcb7211
4aa6308
7445677
534b790
f415c42
1d28b26
95e0fa1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| releases: | ||
| "@yarnpkg/sdks": minor |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import {PortablePath} from '@yarnpkg/fslib'; | ||
| import {npath, PortablePath, ppath} from '@yarnpkg/fslib'; | ||
| import {PnpApi} from '@yarnpkg/pnp'; | ||
|
|
||
| import {Wrapper, GenerateBaseWrapper, BaseSdks} from '../generateSdk'; | ||
|
|
@@ -285,6 +285,69 @@ export const generateFlowBinBaseWrapper: GenerateBaseWrapper = async (pnpApi: Pn | |
| return wrapper; | ||
| }; | ||
|
|
||
| export const generateOxfmtBaseWrapper: GenerateBaseWrapper = async (pnpApi: PnpApi, target: PortablePath) => { | ||
| const wrapper = new Wrapper(`oxfmt` as PortablePath, {pnpApi, target}); | ||
|
|
||
| await wrapper.writeDefaults(); | ||
|
|
||
| return wrapper; | ||
| }; | ||
|
|
||
| export const generateOxlintBaseWrapper: GenerateBaseWrapper = async (pnpApi: PnpApi, target: PortablePath) => { | ||
| const wrapper = new Wrapper(`oxlint` as PortablePath, {pnpApi, target}); | ||
| await wrapper.writeDefaults(); | ||
|
|
||
| // The following workaround is necessary because: | ||
| // 1. oxlint uses top-level await, which prevents the use of `require`. | ||
| // 2. Neither dist/cli.js nor bin/oxlint are exposed as exports in the package.json. | ||
| const topLevelInformation = pnpApi.getPackageInformation(pnpApi.topLevel)!; | ||
| const dependencyReference = topLevelInformation.packageDependencies.get(`oxlint`)!; | ||
| const pkgInformation = pnpApi.getPackageInformation(pnpApi.getLocator(`oxlint`, dependencyReference))!; | ||
| const absPath = pkgInformation.packageLocation; | ||
| const binPath = npath.join(npath.fromPortablePath( | ||
| wrapper.getProjectPathTo(`bin/oxlint` as PortablePath), | ||
| )); | ||
| const relPath = npath.relative(npath.dirname(binPath), absPath); | ||
|
|
||
| // We are using pass-through here since we don't really need to change the default behavior of the wrapper. | ||
| // Since the oxlint wrapper is the one that spawns the actual oxlint binary, we extend the PATH here | ||
| // to enable the tsgolint PATH resolution strategy in the next tsgolint wrapper. | ||
| const oxlintMonkeyPatch = ` | ||
| module => module; | ||
|
|
||
| process.env.PATH += \`;\${resolve(__dirname, '../../oxlint-tsgolint/bin')}\`; | ||
| import(\`${ppath.join(npath.toPortablePath(relPath), `dist/cli.js`)}\`); | ||
| `; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like the import is still necessary even if we change the exports setup and the entrypoint back to bin/oxlint. The top-level await is the blocker :/
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With proper exports from
The more proper fix would be to actually support generating ESM SDK files, but to do so we need Short of that, this stopgap is fine for me for the time being like I said.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Ah, I understand now, thanks! I haven't though that far. It does more natural to resolve via the created require, yeah. I will try to notify upstream package.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
| // This intentionally produces a dummy export; the main logic is in the import above. | ||
| // Originally, the binary does not export anything. | ||
| await wrapper.writeFile(`bin/oxlint` as PortablePath, { | ||
| requirePath: `` as PortablePath, | ||
| wrapModule: oxlintMonkeyPatch, | ||
| }); | ||
|
|
||
| return wrapper; | ||
| }; | ||
|
|
||
| export const generateOxlintTsgolintBaseWrapper: GenerateBaseWrapper = async (pnpApi: PnpApi, target: PortablePath) => { | ||
| const wrapper = new Wrapper(`oxlint-tsgolint` as PortablePath, {pnpApi, target}); | ||
|
|
||
| // We are using the oxc_linter tsgolint resolution mechanism via the PATH environment variable | ||
| // since it's the only realistic approach to correctly resolve the tsgolint binary when using Yarn PnP. | ||
| // Ref: https://github.com/oxc-project/oxc/blob/d3dcf5bc9718ebb4839be27062b5d82da2118e2e/crates/oxc_linter/src/tsgolint.rs#L1164-L1225 | ||
| // With this approach, we need to add a Windows executable shim for tsgolint.js. | ||
| const tsgolintCmd = ` | ||
| @echo off | ||
| node "%~dp0tsgolint.js" %* | ||
| `.trim().replace(/^ {4}/gm, ``); | ||
|
slainless marked this conversation as resolved.
|
||
|
|
||
| await wrapper.writeDefaults(); | ||
| await wrapper.writeFile(`bin/tsgolint` as PortablePath); | ||
| await wrapper.writeRaw(`bin/tsgolint.cmd` as PortablePath, tsgolintCmd); | ||
|
|
||
| return wrapper; | ||
| }; | ||
|
|
||
| export const BASE_SDKS: BaseSdks = [ | ||
| [`@astrojs/language-server`, generateAstroLanguageServerBaseWrapper], | ||
| [`eslint`, generateEslintBaseWrapper], | ||
|
|
@@ -294,4 +357,7 @@ export const BASE_SDKS: BaseSdks = [ | |
| [`typescript`, generateTypescriptBaseWrapper], | ||
| [`svelte-language-server`, generateSvelteLanguageServerBaseWrapper], | ||
| [`flow-bin`, generateFlowBinBaseWrapper], | ||
| [`oxfmt`, generateOxfmtBaseWrapper], | ||
| [`oxlint`, generateOxlintBaseWrapper], | ||
| [`oxlint-tsgolint`, generateOxlintTsgolintBaseWrapper], | ||
| ]; | ||
Uh oh!
There was an error while loading. Please reload this page.