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

chore: migrate fully to Yarn #3155

Merged
merged 6 commits into from
Nov 10, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,8 @@ js/dist/* -diff
js/dist/* linguist-generated
js/dist-typings/* linguist-generated

# https://yarnpkg.com/getting-started/qa#which-files-should-be-gitignored
/js/.yarn/releases/** binary
/js/.yarn/plugins/** binary

* text=auto eol=lf
20 changes: 10 additions & 10 deletions .github/workflows/js.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,15 @@ jobs:
uses: actions/setup-node@v2
with:
node-version: ${{ env.NODE_VERSION }}
cache: "npm"
cache-dependency-path: js/package-lock.json
cache: "yarn"
cache-dependency-path: js/yarn.lock

- name: Install JS dependencies
run: npm ci
run: yarn install --immutable
working-directory: ./js

- name: Check JS formatting
run: npm run format-check
run: yarn run format-check
working-directory: ./js

build-prod:
Expand All @@ -46,8 +46,8 @@ jobs:
uses: actions/setup-node@v2
with:
node-version: ${{ env.NODE_VERSION }}
cache: "npm"
cache-dependency-path: js/package-lock.json
cache: "yarn"
cache-dependency-path: js/yarn.lock

# Our action will install npm, cd into `./js`, run `npm run build` and
# `npm run build-typings`, then commit and upload any changes
Expand All @@ -56,7 +56,7 @@ jobs:
with:
github_token: ${{ secrets.GITHUB_TOKEN }}
build_script: build
package_manager: npm
package_manager: yarn
typings_script: build-typings

build-test:
Expand All @@ -76,8 +76,8 @@ jobs:
uses: actions/setup-node@v2
with:
node-version: ${{ env.NODE_VERSION }}
cache: "npm"
cache-dependency-path: js/package-lock.json
cache: "yarn"
cache-dependency-path: js/yarn.lock

# Our action will install npm, cd into `./js`, run `npm run build` and
# `npm run build-typings`, then commit and upload any changes
Expand All @@ -86,6 +86,6 @@ jobs:
with:
github_token: ${{ secrets.GITHUB_TOKEN }}
build_script: build
package_manager: npm
package_manager: yarn
typings_script: build-typings
do_not_commit: true
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
/vendor
composer.lock
composer.phar
node_modules
.DS_Store
Thumbs.db
tests/.phpunit.result.cache
Expand Down
9 changes: 9 additions & 0 deletions js/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# https://yarnpkg.com/getting-started/qa#which-files-should-be-gitignored
davwheat marked this conversation as resolved.
Show resolved Hide resolved
# We are not using zero-installs
.pnp.*
.yarn/*
!.yarn/patches
!.yarn/plugins
!.yarn/releases
!.yarn/sdks
!.yarn/versions
768 changes: 768 additions & 0 deletions js/.yarn/releases/yarn-3.1.0.cjs

Large diffs are not rendered by default.

5 changes: 5 additions & 0 deletions js/.yarn/sdks/integrations.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# This file is automatically generated by @yarnpkg/sdks.
# Manual changes might be lost!

integrations:
- vscode
20 changes: 20 additions & 0 deletions js/.yarn/sdks/prettier/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#!/usr/bin/env node

const {existsSync} = require(`fs`);
const {createRequire, createRequireFromPath} = require(`module`);
const {resolve} = require(`path`);

const relPnpApiPath = "../../../.pnp.cjs";

const absPnpApiPath = resolve(__dirname, relPnpApiPath);
const absRequire = (createRequire || createRequireFromPath)(absPnpApiPath);

if (existsSync(absPnpApiPath)) {
if (!process.versions.pnp) {
// Setup the environment to be able to require prettier/index.js
require(absPnpApiPath).setup();
}
}

// Defer to the real prettier/index.js your application uses
module.exports = absRequire(`prettier/index.js`);
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Will we need this in every single repo that uses prettier?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but it's only needed for Prettier and TS integration with VSCode and other IDEs.

Imo I think this is worth committing because it creates a working out of the box dev environment.

The code can be updated automatically vwith yarn dlx @yarnpkg/sdks, though the VS Code config file needs to be altered slightly in order to play nicely with our js folder being a subdirectory.

https://yarnpkg.com/getting-started/editor-sdks

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

My concern is keeping this up to date across our (many) repos. What manual changes are needed, and could we automate them somehow?

Copy link
Member Author

Choose a reason for hiding this comment

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

By default, it generates a vscode config file in the JS folder. We just move that out, and prepend js/ to any paths needed.

Looking now, I can't actually see the vscode config file in this PR... I need to add that.

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

By default, it generates a vscode config file in the JS folder. We just move that out, and prepend js/ to any paths needed.

Looking now, I can't actually see the vscode config file in this PR... I need to add that.

I suppose we could add something to flarum-cli. But I wonder if the huge pile of boilerplate we need to keep track of is worth it?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's one of the things that won't be a 'huge pile' when we go monorepo eventually. Our root workspace could have Prettier installed with this boilerplate, and everything would work fine in subdirectories.

Copy link
Sponsor Member

@askvortsov1 askvortsov1 Nov 8, 2021

Choose a reason for hiding this comment

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

It's one of the things that won't be a 'huge pile' when we go monorepo eventually. Our root workspace could have Prettier installed with this boilerplate, and everything would work fine in subdirectories.

It's not just us: monorepo or not, we can't sacrifice developer experience for third party devs.

I know these comments aren't particularly constructive. But I guess my point is that I'm not sure that Yarn 2 is worth it atm. It seems like most major players are still in the npm / yarn 1 system, and that npm itself is getting a lot better and more scalable.

Copy link
Member Author

Choose a reason for hiding this comment

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

Just a note that these changes you're referring to are specifically for Yarn PnP support rather than Yarn 2 or Yarn 3.

Reverting 9781c8c will let us use Yarn 3 without PnP using the legacy node_modules linker.

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

To be honest, if we want to commit entirely to Yarn, I would prefer switching to Yarn 3 without PnP, and then reconsider PnP at a later time. Incremental steps!

6 changes: 6 additions & 0 deletions js/.yarn/sdks/prettier/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "prettier",
"version": "2.4.1-sdk",
"main": "./index.js",
"type": "commonjs"
}
20 changes: 20 additions & 0 deletions js/.yarn/sdks/typescript/bin/tsc
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#!/usr/bin/env node

const {existsSync} = require(`fs`);
const {createRequire, createRequireFromPath} = require(`module`);
const {resolve} = require(`path`);

const relPnpApiPath = "../../../../.pnp.cjs";

const absPnpApiPath = resolve(__dirname, relPnpApiPath);
const absRequire = (createRequire || createRequireFromPath)(absPnpApiPath);

if (existsSync(absPnpApiPath)) {
if (!process.versions.pnp) {
// Setup the environment to be able to require typescript/bin/tsc
require(absPnpApiPath).setup();
}
}

// Defer to the real typescript/bin/tsc your application uses
module.exports = absRequire(`typescript/bin/tsc`);
20 changes: 20 additions & 0 deletions js/.yarn/sdks/typescript/bin/tsserver
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#!/usr/bin/env node

const {existsSync} = require(`fs`);
const {createRequire, createRequireFromPath} = require(`module`);
const {resolve} = require(`path`);

const relPnpApiPath = "../../../../.pnp.cjs";

const absPnpApiPath = resolve(__dirname, relPnpApiPath);
const absRequire = (createRequire || createRequireFromPath)(absPnpApiPath);

if (existsSync(absPnpApiPath)) {
if (!process.versions.pnp) {
// Setup the environment to be able to require typescript/bin/tsserver
require(absPnpApiPath).setup();
}
}

// Defer to the real typescript/bin/tsserver your application uses
module.exports = absRequire(`typescript/bin/tsserver`);
20 changes: 20 additions & 0 deletions js/.yarn/sdks/typescript/lib/tsc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#!/usr/bin/env node

const {existsSync} = require(`fs`);
const {createRequire, createRequireFromPath} = require(`module`);
const {resolve} = require(`path`);

const relPnpApiPath = "../../../../.pnp.cjs";

const absPnpApiPath = resolve(__dirname, relPnpApiPath);
const absRequire = (createRequire || createRequireFromPath)(absPnpApiPath);

if (existsSync(absPnpApiPath)) {
if (!process.versions.pnp) {
// Setup the environment to be able to require typescript/lib/tsc.js
require(absPnpApiPath).setup();
}
}

// Defer to the real typescript/lib/tsc.js your application uses
module.exports = absRequire(`typescript/lib/tsc.js`);
184 changes: 184 additions & 0 deletions js/.yarn/sdks/typescript/lib/tsserver.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,184 @@
#!/usr/bin/env node

const {existsSync} = require(`fs`);
const {createRequire, createRequireFromPath} = require(`module`);
const {resolve} = require(`path`);

const relPnpApiPath = "../../../../.pnp.cjs";

const absPnpApiPath = resolve(__dirname, relPnpApiPath);
const absRequire = (createRequire || createRequireFromPath)(absPnpApiPath);

const moduleWrapper = tsserver => {
if (!process.versions.pnp) {
return tsserver;
}

const {isAbsolute} = require(`path`);
const pnpApi = require(`pnpapi`);

const isVirtual = str => str.match(/\/(\$\$virtual|__virtual__)\//);
const normalize = str => str.replace(/\\/g, `/`).replace(/^\/?/, `/`);

const dependencyTreeRoots = new Set(pnpApi.getDependencyTreeRoots().map(locator => {
return `${locator.name}@${locator.reference}`;
}));

// VSCode sends the zip paths to TS using the "zip://" prefix, that TS
// doesn't understand. This layer makes sure to remove the protocol
// before forwarding it to TS, and to add it back on all returned paths.

function toEditorPath(str) {
// We add the `zip:` prefix to both `.zip/` paths and virtual paths
if (isAbsolute(str) && !str.match(/^\^?(zip:|\/zip\/)/) && (str.match(/\.zip\//) || isVirtual(str))) {
// We also take the opportunity to turn virtual paths into physical ones;
// this makes it much easier to work with workspaces that list peer
// dependencies, since otherwise Ctrl+Click would bring us to the virtual
// file instances instead of the real ones.
//
// We only do this to modules owned by the the dependency tree roots.
// This avoids breaking the resolution when jumping inside a vendor
// with peer dep (otherwise jumping into react-dom would show resolution
// errors on react).
//
const resolved = isVirtual(str) ? pnpApi.resolveVirtual(str) : str;
if (resolved) {
const locator = pnpApi.findPackageLocator(resolved);
if (locator && dependencyTreeRoots.has(`${locator.name}@${locator.reference}`)) {
str = resolved;
}
}

str = normalize(str);

if (str.match(/\.zip\//)) {
switch (hostInfo) {
// Absolute VSCode `Uri.fsPath`s need to start with a slash.
// VSCode only adds it automatically for supported schemes,
// so we have to do it manually for the `zip` scheme.
// The path needs to start with a caret otherwise VSCode doesn't handle the protocol
//
// Ref: https://github.com/microsoft/vscode/issues/105014#issuecomment-686760910
//
// Update Oct 8 2021: VSCode changed their format in 1.61.
// Before | ^zip:/c:/foo/bar.zip/package.json
// After | ^/zip//c:/foo/bar.zip/package.json
//
case `vscode <1.61`: {
str = `^zip:${str}`;
} break;

case `vscode`: {
str = `^/zip/${str}`;
} break;

// To make "go to definition" work,
// We have to resolve the actual file system path from virtual path
// and convert scheme to supported by [vim-rzip](https://github.com/lbrayner/vim-rzip)
case `coc-nvim`: {
str = normalize(resolved).replace(/\.zip\//, `.zip::`);
str = resolve(`zipfile:${str}`);
} break;

// Support neovim native LSP and [typescript-language-server](https://github.com/theia-ide/typescript-language-server)
// We have to resolve the actual file system path from virtual path,
// everything else is up to neovim
case `neovim`: {
str = normalize(resolved).replace(/\.zip\//, `.zip::`);
str = `zipfile:${str}`;
} break;

default: {
str = `zip:${str}`;
} break;
}
}
}

return str;
}

function fromEditorPath(str) {
switch (hostInfo) {
case `coc-nvim`:
case `neovim`: {
str = str.replace(/\.zip::/, `.zip/`);
// The path for coc-nvim is in format of /<pwd>/zipfile:/<pwd>/.yarn/...
// So in order to convert it back, we use .* to match all the thing
// before `zipfile:`
return process.platform === `win32`
? str.replace(/^.*zipfile:\//, ``)
: str.replace(/^.*zipfile:/, ``);
} break;

case `vscode`:
default: {
return process.platform === `win32`
? str.replace(/^\^?(zip:|\/zip)\/+/, ``)
: str.replace(/^\^?(zip:|\/zip)\/+/, `/`);
} break;
}
}

// Force enable 'allowLocalPluginLoads'
// TypeScript tries to resolve plugins using a path relative to itself
// which doesn't work when using the global cache
// https://github.com/microsoft/TypeScript/blob/1b57a0395e0bff191581c9606aab92832001de62/src/server/project.ts#L2238
// VSCode doesn't want to enable 'allowLocalPluginLoads' due to security concerns but
// TypeScript already does local loads and if this code is running the user trusts the workspace
// https://github.com/microsoft/vscode/issues/45856
const ConfiguredProject = tsserver.server.ConfiguredProject;
const {enablePluginsWithOptions: originalEnablePluginsWithOptions} = ConfiguredProject.prototype;
ConfiguredProject.prototype.enablePluginsWithOptions = function() {
this.projectService.allowLocalPluginLoads = true;
return originalEnablePluginsWithOptions.apply(this, arguments);
};

// And here is the point where we hijack the VSCode <-> TS communications
// by adding ourselves in the middle. We locate everything that looks
// like an absolute path of ours and normalize it.

const Session = tsserver.server.Session;
const {onMessage: originalOnMessage, send: originalSend} = Session.prototype;
let hostInfo = `unknown`;

Object.assign(Session.prototype, {
onMessage(/** @type {string} */ message) {
const parsedMessage = JSON.parse(message)

if (
parsedMessage != null &&
typeof parsedMessage === `object` &&
parsedMessage.arguments &&
typeof parsedMessage.arguments.hostInfo === `string`
) {
hostInfo = parsedMessage.arguments.hostInfo;
if (hostInfo === `vscode` && process.env.VSCODE_IPC_HOOK && process.env.VSCODE_IPC_HOOK.match(/Code\/1\.([1-5][0-9]|60)\./)) {
hostInfo += ` <1.61`;
}
}

return originalOnMessage.call(this, JSON.stringify(parsedMessage, (key, value) => {
return typeof value === `string` ? fromEditorPath(value) : value;
}));
},

send(/** @type {any} */ msg) {
return originalSend.call(this, JSON.parse(JSON.stringify(msg, (key, value) => {
return typeof value === `string` ? toEditorPath(value) : value;
})));
}
});

return tsserver;
};

if (existsSync(absPnpApiPath)) {
if (!process.versions.pnp) {
// Setup the environment to be able to require typescript/lib/tsserver.js
require(absPnpApiPath).setup();
}
}

// Defer to the real typescript/lib/tsserver.js your application uses
module.exports = moduleWrapper(absRequire(`typescript/lib/tsserver.js`));
Loading