Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Implement Yarn 2 support #1630

Closed
wants to merge 15 commits into from
Closed
Show file tree
Hide file tree
Changes from 7 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
52 changes: 52 additions & 0 deletions packages/eslint/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,58 @@ The following is a list of rules and their identifiers which can be overridden:
| ------ | ------------------------------------------------------------------------------------------------------------------------------- | -------- |
| `lint` | By default, lints JS and JSX files from the `src` and `test` directories using ESLint. Contains a single loader named `eslint`. | all |

## Utility functions

### Plugins aliasing

When developing your custom ESLint preset, you may face a problem with sharable ESLint configs and plugins. This is due to ESLint plugins resolving system which searches for plugins packages in the project root. This may fail in some environments, especially in Plug'n'Play. This module provides `aliasPlugins()` function to resolve this issue in your package. You can import it in 2 ways: `require('@neutrinojs/eslint/alias-plugins')` or `require('@neutrinojs/eslint').aliasPlugins`. Example:

```js
const eslint = require('@neutrinojs/eslint');
const eslintBaseConfig = { plugins: ['node'] };

neutrino.use(eslint({
eslint: {
baseConfig: eslintBaseConfig
}
}))

lint.aliasPlugins(
// ESLint config that contains used plugins
eslintBaseConfig,
// Path to the current module file, so aliases can be correctly resolved relative to your package
// In most cases it is always `__filename`
__filename
);
```

If you use 3rd party configs, plugins will not be present in the configuration. So you have to list them manually just for aliasing. For example:

```js
const eslint = require('@neutrinojs/eslint');
const usedPlugins = ['react', 'react-hooks', 'jsx-a11y', 'import'];
const eslintBaseConfig = {
extends: [
require.resolve('eslint-config-airbnb'),
require.resolve('eslint-config-airbnb/hooks')
]
};

neutrino.use(eslint({
eslint: {
baseConfig: eslintBaseConfig
}
}))

lint.aliasPlugins(
// ESLint config that contains only used plugins
{ plugins: usedPlugins },
// Path to the current module file, so aliases can be correctly resolved relative to your package
// In most cases it is always `__filename`
__filename
);
```

## Contributing

This middleware is part of the
Expand Down
41 changes: 41 additions & 0 deletions packages/eslint/alias-plugins.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
const moduleAlias = require('module-alias');
const pnpApi = process.versions.pnp ? require('pnpapi') : null; // eslint-disable-line import/no-unresolved

function toFullName(pluginName) {
const ESLINT_PREFIX = 'eslint-plugin-';
const ORGANIZATION_EXPRESSION = /^(@[\d.A-z-]+)\/(.+)$/;
const nameIsFull = pluginName.indexOf(ESLINT_PREFIX) === 0;
const nameIsOrganization = ORGANIZATION_EXPRESSION.test(pluginName);

if (nameIsOrganization) {
const [, organizationName, name] = pluginName.match(
ORGANIZATION_EXPRESSION,
);

return `${organizationName}/${toFullName(name)}`;
}

return nameIsFull ? pluginName : `${ESLINT_PREFIX}${pluginName}`;
}

function aliasModuleFrom(relativeFilename = __filename) {
return function aliasPlugin(pluginName) {
let resolvedPluginPath;

if (pnpApi) {
resolvedPluginPath = pnpApi.resolveRequest(pluginName, relativeFilename);
} else {
resolvedPluginPath = require.resolve(pluginName, {
paths: [relativeFilename],
});
}
Copy link

Choose a reason for hiding this comment

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

You should just use require.resolve you almost never need to use the pnpapi directly, this is one of those cases

Copy link
Contributor Author

@constgen constgen Nov 3, 2020

Choose a reason for hiding this comment

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

I know. But unfortunately __filename gives a virtual File System path in Yarn 2 which can't work correctly in paths options of require.resolve. Module will be not found with an exception. Struggled with this 2 days.

Copy link

Choose a reason for hiding this comment

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

Do you have a reproduction for that? We have tests making sure it works. I could see that happening if you have enableGlobalCache: true but that will be fixed in yarnpkg/berry#2068

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In any place of your code in Yarn 2 environment try this console.log(require.resolve('any-of-your-dependency', { paths: [__filename] })). It will not find your module even if it is relative from the current file. The same code works without any problems in regular NPM environment

Copy link

@merceyz merceyz Nov 3, 2020

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@constgen constgen Nov 6, 2020

Choose a reason for hiding this comment

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

I realized that the mentioned test case works in the Yarn 2 project itself. But in the linked module it fails with not found dependency. As you understand I use linked packages for local environments. Such modules are defined as

{
 "devDependencies": {
   "@neutrinojs/eslint": "portal:C:/Projects/Projects_GitHub/neutrino-dev-fork/packages/eslint",
  }
}

yarn install is called to setup all links in .yarn/cache and .pnp.js
And after that I can run local script "lint": "eslint ./src --ext .js,.jsx", to run the code and test the case

Copy link

Choose a reason for hiding this comment

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

Yeah, that makes more sense. I'm fairly certain the issue you're running into was fixed in yarnpkg/berry#2068, could you try with yarn set version from sources or by downloading the bundle here https://github.com/yarnpkg/berry/actions/runs/346381133?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before: Yarn version v2.2.2
After: Yarn version 2.3.3-git.20201112.f5ca1c57
Result is the same in the linked package: works when require.resolve('babel-eslint') and fails when require.resolve('babel-eslint', { paths: [__filename] })

Oops! Something went wrong! :(

ESLint: 7.9.0

Error: Cannot read config file: C:\Users\const\Desktop\yarn-test\.eslintrc.js
Error: Cannot find module 'babel-eslint'

Copy link

Choose a reason for hiding this comment

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

Could you try using __dirname instead?
Again, if you could provide a reproduction I'll be able to look into it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update:
After removing .yarn/cache and reinstalling dependencies require.resolve('babel-eslint', { paths: [__filename] }) started to work. It was the version 2.3.3-git.20201112.f5ca1c57.
Than I upgraded Yarn to the latest 2.4.0 and reinstalled dependencies again. And it still works. So I removed usage of PnP API as you asked


moduleAlias.addAlias(pluginName, resolvedPluginPath);
};
}

module.exports = function aliasPlugins(eslintConfig, relativeFilename) {
const { plugins = [] } = eslintConfig;

plugins.map(toFullName).forEach(aliasModuleFrom(relativeFilename));
};
9 changes: 7 additions & 2 deletions packages/eslint/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const { ConfigurationError, DuplicateRuleError } = require('neutrino/errors');
const aliasPlugins = require('./alias-plugins');

const arrayToObject = (array) =>
array.reduce((obj, item) => Object.assign(obj, { [item]: true }), {});
Expand Down Expand Up @@ -125,7 +126,7 @@ module.exports = ({ test, include, exclude, eslint = {} } = {}) => {
}

const baseConfig = eslint.baseConfig || {};

const usedPlugins = ['babel'];
const loaderOptions = {
// For supported options, see:
// https://github.com/webpack-contrib/eslint-loader#options
Expand Down Expand Up @@ -172,10 +173,12 @@ module.exports = ({ test, include, exclude, eslint = {} } = {}) => {
// Unfortunately we can't `require.resolve('eslint-plugin-babel')` due to:
// https://github.com/eslint/eslint/issues/6237
// ...so we have no choice but to rely on it being hoisted.
plugins: ['babel', ...(baseConfig.plugins || [])],
plugins: [...usedPlugins, ...(baseConfig.plugins || [])],
},
};

aliasPlugins({ plugins: usedPlugins });

neutrino.config.module
.rule('lint')
.test(test || neutrino.regexFromExtensions())
Expand All @@ -192,3 +195,5 @@ module.exports = ({ test, include, exclude, eslint = {} } = {}) => {
neutrino.register('eslintrc', eslintrc);
};
};

module.exports.aliasPlugins = aliasPlugins;
3 changes: 2 additions & 1 deletion packages/eslint/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@
"dependencies": {
"babel-eslint": "^10.1.0",
"eslint-loader": "^4.0.2",
"eslint-plugin-babel": "^5.3.1"
"eslint-plugin-babel": "^5.3.1",
"module-alias": "^2.2.2"
},
"peerDependencies": {
"eslint": "^6.0.0 || ^7.0.0",
Expand Down
4 changes: 1 addition & 3 deletions packages/jest/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,7 @@ module.exports = (options = {}) => (neutrino) => {
return path;
}

return path.startsWith('.')
? join('<rootDir>', path)
: join('<rootDir>', 'node_modules', path);
return path.startsWith('.') ? join('<rootDir>', path) : path;
};
const extensionsToNames = (extensions) => `\\.(${extensions.join('|')})$`;
const { extensions, source, tests, root, debug } = neutrino.options;
Expand Down
29 changes: 28 additions & 1 deletion packages/jest/test/jest_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ test('exposes babel config without babel-loader specific options', (t) => {
t.false('customize' in babelOptions);
});

test('configures webpack aliases in moduleNameMapper correctly', (t) => {
test('configures absolute webpack aliases in moduleNameMapper correctly', (t) => {
const api = new Neutrino();
const reactPath = path.resolve(path.join('node_modules', 'react'));
api.use(reactPreset());
Expand All @@ -174,3 +174,30 @@ test('configures webpack aliases in moduleNameMapper correctly', (t) => {
}),
);
});

test('configures package webpack aliases in moduleNameMapper correctly', (t) => {
const api = new Neutrino();
api.use(mw());
api.config.resolve.alias.set('_', 'lodash');
const config = api.outputHandlers.get('jest')(api);

t.true(
Object.entries(config.moduleNameMapper).some(([key, alias]) => {
return key === '^_$' && alias === 'lodash';
}),
);
});

test('configures relative webpack aliases in moduleNameMapper correctly', (t) => {
const api = new Neutrino();
api.use(mw());
api.config.resolve.alias.set('images', './src/images');
const config = api.outputHandlers.get('jest')(api);

t.true(
Object.entries(config.moduleNameMapper).some(([key, alias]) => {
const urlAlias = alias.replace(/\\/g, '/'); // consider OS difference
return key === '^images$' && urlAlias === '<rootDir>/src/images';
}),
);
});
2 changes: 2 additions & 0 deletions packages/library/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const banner = require('@neutrinojs/banner');
const compileLoader = require('@neutrinojs/compile-loader');
const clean = require('@neutrinojs/clean');
const pnp = require('@neutrinojs/pnp');
const babelMerge = require('babel-merge');
const merge = require('deepmerge');
const nodeExternals = require('webpack-node-externals');
Expand Down Expand Up @@ -63,6 +64,7 @@ module.exports = (opts = {}) => {
(pkg.dependencies && 'source-map-support' in pkg.dependencies) ||
(pkg.devDependencies && 'source-map-support' in pkg.devDependencies);

neutrino.use(pnp());
neutrino.use(
compileLoader({
include: [neutrino.options.source, neutrino.options.tests],
Expand Down
1 change: 1 addition & 0 deletions packages/library/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
"@neutrinojs/banner": "9.4.0",
"@neutrinojs/clean": "9.4.0",
"@neutrinojs/compile-loader": "9.4.0",
"@neutrinojs/pnp": "9.4.0",
"babel-merge": "^3.0.0",
"deepmerge": "^1.5.2",
"webpack-node-externals": "^1.7.2"
Expand Down
2 changes: 2 additions & 0 deletions packages/node/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ const banner = require('@neutrinojs/banner');
const compileLoader = require('@neutrinojs/compile-loader');
const clean = require('@neutrinojs/clean');
const startServer = require('@neutrinojs/start-server');
const pnp = require('@neutrinojs/pnp');
const babelMerge = require('babel-merge');
const nodeExternals = require('webpack-node-externals');
const { basename, parse, format } = require('path');
Expand Down Expand Up @@ -39,6 +40,7 @@ module.exports = (opts = {}) => {
);
const coreJsVersion = neutrino.getDependencyVersion('core-js');

neutrino.use(pnp());
neutrino.use(
compileLoader({
include: [neutrino.options.source, neutrino.options.tests],
Expand Down
1 change: 1 addition & 0 deletions packages/node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
"@neutrinojs/banner": "9.4.0",
"@neutrinojs/clean": "9.4.0",
"@neutrinojs/compile-loader": "9.4.0",
"@neutrinojs/pnp": "9.4.0",
"@neutrinojs/start-server": "9.4.0",
"babel-merge": "^3.0.0",
"deepmerge": "^1.5.2",
Expand Down
1 change: 1 addition & 0 deletions packages/pnp/.npmignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/test/
87 changes: 87 additions & 0 deletions packages/pnp/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
# Neutrino Plug'n'Play Middleware

`@neutrinojs/pnp` is Neutrino middleware for Plug'n'Play support.

[![NPM version][npm-image]][npm-url] [![NPM downloads][npm-downloads]][npm-url]

## Requirements

- Node.js 10+
- Yarn v1.2.1+, or npm v5.4+
- Neutrino 9
- Webpack 4

## Installation

`@neutrinojs/pnp` can be installed via the Yarn or npm clients.

#### Yarn

```bash
❯ yarn add --dev @neutrinojs/pnp
```

#### npm

```bash
❯ npm install --save-dev @neutrinojs/pnp
```

## Usage

`@neutrinojs/pnp` can be consumed from the Neutrino API, middleware, or presets.
Require this package and plug it into Neutrino:

```js
const pnp = require('@neutrinojs/pnp');

// Use with default options
neutrino.use(pnp());

// Usage shows the default values of this middleware:
neutrino.use(pnp({ pluginId: 'pnp' }));
```

```js
// Using in .neutrinorc.js
const pnp = require('@neutrinojs/pnp');

// Use with default options
module.exports = {
use: [pnp()],
};

// Usage shows the default values of this middleware:
module.exports = {
use: [pnp({ pluginId: 'pnp' })],
};
```

- `pluginId`: The plugin identifier. Override this to add an additional copy
plugin instance.

## Customization

`@neutrinojs/pnp` creates some conventions to make overriding the configuration
easier once you are ready to make changes.

### Plugins

The following is a list of plugins and their identifiers which can be
overridden:

| Name | Description | NODE_ENV |
| ----- | --------------------------------------------------------- | -------- |
| `pnp` | Resolve modules considering Plug and Play feature of Yarn | all |

## Contributing

This middleware is part of the
[neutrino](https://github.com/neutrinojs/neutrino) repository, a monorepo
containing all resources for developing Neutrino and its core presets and
middleware. Follow the
[contributing guide](https://neutrinojs.org/contributing/) for details.

[npm-image]: https://img.shields.io/npm/v/@neutrinojs/pnp.svg
[npm-downloads]: https://img.shields.io/npm/dt/@neutrinojs/pnp.svg
[npm-url]: https://www.npmjs.com/package/@neutrinojs/pnp
41 changes: 41 additions & 0 deletions packages/pnp/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
const moduleAlias = require('module-alias');

module.exports = function pnp(settings = {}) {
return function pnpMiddleware(neutrino) {
const { pluginId = 'pnp' } = settings;
const projectPath = process.cwd();
const environmentIsPnP = Boolean(process.versions.pnp);

if (environmentIsPnP) {
// solve the issue with the linked middleware outside of the project root
// https://github.com/yarnpkg/berry/issues/693
moduleAlias.addAlias(
'pnpapi',
require.resolve('pnpapi', { paths: [projectPath] }),
);
}

// eslint-disable-next-line global-require -- Have to import this after `require` alias is applied
const PnpWebpackPlugin = require(`pnp-webpack-plugin`);

function PnpPlugin() {
return PnpWebpackPlugin;
}
function PnpLoaderPlugin() {
return PnpWebpackPlugin.moduleLoader(module);
}

/* eslint-disable prettier/prettier */
neutrino.config
.resolve
.plugin(pluginId)
.use(PnpPlugin)
.end()
.end()
.resolveLoader
.plugin(pluginId)
.use(PnpLoaderPlugin)
.end()
.end();
};
};
Loading