Skip to content

Commit

Permalink
Merge pull request #188 from embroider-build/set-npmrc
Browse files Browse the repository at this point in the history
pnpm: suggest stricter dep management
  • Loading branch information
NullVoxPopuli authored Aug 19, 2023
2 parents 4acd7fd + 73505c3 commit 5b90f8b
Show file tree
Hide file tree
Showing 10 changed files with 53 additions and 14 deletions.
9 changes: 9 additions & 0 deletions files/.npmrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Docs: https://pnpm.io/npmrc
# https://github.com/emberjs/rfcs/pull/907

# we don't want addons to be bad citizens of the ecosystem
auto-install-peers=false

# we want true isolation,
# if a dependency is not declared, we want an error
resolve-peers-from-workspace-root=false
9 changes: 9 additions & 0 deletions files/__addonLocation__/.npmrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Docs: https://pnpm.io/npmrc
# https://github.com/emberjs/rfcs/pull/907

# we don't want addons to be bad citizens of the ecosystem
auto-install-peers=false

# we want true isolation,
# if a dependency is not declared, we want an error
resolve-peers-from-workspace-root=false
7 changes: 7 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,10 @@ module.exports = {

if (options.addonOnly) {
files = files.filter((filename) => filename.includes('__addonLocation__'));
} else {
// filter out the addon-specific npmrc, as it
// is only applicable during --addon-only
files = files.filter((filename) => !filename.includes('__addonLocation__/.npmrc'));
}

if (!options.typescript) {
Expand All @@ -314,6 +318,9 @@ module.exports = {
// The .gitignore is used for ignoring files that are moved to the addon from the root folder at build time
// But this setup does not apply for an existing monorepo (all root files are ignored), so we don't need the .gitignore
files = files.filter((filename) => filename !== '__addonLocation__/gitignore');
// In an existing monorepo, we typically have a single .npmrc for the whole repo.
// We don't want to generate an .npmrc for those situations.
files = files.filter((filename) => !filename.endsWith('.npmrc'));
}

return files;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { hbs } from 'ember-cli-htmlbars';

import * as myModule from 'my-addon';

module('imports', function (hooks) {
module('imports', function () {
test('did they work', async function (assert) {
assert.ok(myModule.MyService);
assert.ok(myModule.JSComponent);
Expand Down
9 changes: 9 additions & 0 deletions tests/fixtures/pnpm-addon-only/.npmrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Docs: https://pnpm.io/npmrc
# https://github.com/emberjs/rfcs/pull/907

# we don't want addons to be bad citizens of the ecosystem
auto-install-peers=false

# we want true isolation,
# if a dependency is not declared, we want an error
resolve-peers-from-workspace-root=false
9 changes: 9 additions & 0 deletions tests/fixtures/pnpm/.npmrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Docs: https://pnpm.io/npmrc
# https://github.com/emberjs/rfcs/pull/907

# we don't want addons to be bad citizens of the ecosystem
auto-install-peers=false

# we want true isolation,
# if a dependency is not declared, we want an error
resolve-peers-from-workspace-root=false
11 changes: 0 additions & 11 deletions tests/helpers/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import path from 'node:path';
import { fileURLToPath } from 'node:url';

import { execa,type Options } from 'execa';
import fse from 'fs-extra';

const DEBUG = process.env.DEBUG === 'true';
const __dirname = path.dirname(fileURLToPath(import.meta.url));
Expand Down Expand Up @@ -143,15 +142,5 @@ export async function createAddon({
preferLocal: true,
});

// Light work-around for an upstream `@babel/core` peer issue
if (typeof options.cwd === 'string') {
await fs.writeFile(
fse.existsSync(path.join(options.cwd, name))
? path.join(options.cwd, name, '.npmrc')
: path.join(options.cwd, '.npmrc'),
'auto-install-peers=true'
);
}

return { result, name };
}
5 changes: 4 additions & 1 deletion tests/smoke-tests/--addon-only.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import path from 'node:path';
import fse from 'fs-extra';
import { afterAll, beforeAll, describe, expect, it } from 'vitest';

import { AddonHelper, dirContents } from '../helpers.js';
import { AddonHelper, dirContents, matchesFixture } from '../helpers.js';

describe('--addon-only', () => {
let helper = new AddonHelper({ packageManager: 'pnpm', args: ['--addon-only'] });
Expand All @@ -20,6 +20,9 @@ describe('--addon-only', () => {
it('has all the dot files', async () => {
let contents = await dirContents(helper.projectRoot);

await matchesFixture('.npmrc', { cwd: helper.projectRoot, scenario: 'pnpm-addon-only' });

expect(contents).to.include('.npmrc');
expect(contents).to.include('.eslintrc.cjs');
expect(contents).to.include('.eslintignore');
expect(contents).to.include('.prettierrc.cjs');
Expand Down
4 changes: 4 additions & 0 deletions tests/smoke-tests/defaults.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ for (let packageManager of SUPPORTED_PACKAGE_MANAGERS) {
scenario: 'pnpm',
});
await matchesFixture('CONTRIBUTING.md', { cwd: helper.projectRoot, scenario: 'pnpm' });
await matchesFixture('.npmrc', {
cwd: helper.projectRoot,
scenario: 'pnpm',
});

expect(testManifest.devDependencies['my-addon']).toBe('workspace:*');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ for (let packageManager of SUPPORTED_PACKAGE_MANAGERS) {
it('ignores root files', async () => {
expect(
fixturify.readSync(cwd, {
ignore: ['my-addon', 'test-app', 'node_modules', lockFile, '.npmrc'],
ignore: ['my-addon', 'test-app', 'node_modules', lockFile],
}),
'root files have not been touched'
).toEqual(rootFiles);
Expand Down

0 comments on commit 5b90f8b

Please sign in to comment.