Skip to content

Commit

Permalink
feat: remove node.import ESM CJS wrapper support
Browse files Browse the repository at this point in the history
  • Loading branch information
@stipsan
stipsan committed Mar 7, 2024
1 parent f79c63d commit d481e4a
Show file tree
Hide file tree
Showing 11 changed files with 42 additions and 106 deletions.
7 changes: 2 additions & 5 deletions playground/default-export/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,12 @@
"private": true,
"license": "MIT",
"type": "module",
"sideEffects": false,
"exports": {
".": {
"source": "./src/index.js",
"require": "./dist/index.cjs",
"node": {
"module": "./dist/index.js",
"import": "./dist/index.cjs.js"
},
"import": "./dist/index.js",
"require": "./dist/index.cjs",
"default": "./dist/index.js"
},
"./package.json": "./package.json"
Expand Down
2 changes: 0 additions & 2 deletions playground/dummy-commonjs/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
"require": "./dist/index.browser.js"
},
"node": {
"module": "./dist/index.mjs",
"import": "./node/index.mjs",
"require": "./node/index.js"
},
Expand All @@ -31,7 +30,6 @@
"require": "./dist/extra.browser.js"
},
"node": {
"module": "./dist/extra.mjs",
"import": "./node/extra.mjs",
"require": "./node/extra.js"
},
Expand Down
2 changes: 0 additions & 2 deletions playground/dummy-module/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
"require": "./dist/index.browser.cjs"
},
"node": {
"module": "./dist/index.js",
"import": "./node/index.js",
"require": "./node/index.cjs"
},
Expand All @@ -31,7 +30,6 @@
"require": "./dist/extra.browser.cjs"
},
"node": {
"module": "./dist/extra.js",
"import": "./node/extra.js",
"require": "./node/extra.cjs"
},
Expand Down
12 changes: 2 additions & 10 deletions playground/multi-export/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,15 @@
".": {
"types": "./dist/index.d.ts",
"source": "./src/index.ts",
"require": "./dist/index.cjs",
"node": {
"module": "./dist/index.js",
"import": "./dist/index.cjs.js"
},
"import": "./dist/index.js",
"require": "./dist/index.cjs",
"default": "./dist/index.js"
},
"./plugin": {
"types": "./dist/plugin.d.ts",
"source": "./src/plugin.ts",
"require": "./dist/plugin.cjs",
"node": {
"module": "./dist/plugin.js",
"import": "./dist/plugin.cjs.js"
},
"import": "./dist/plugin.js",
"require": "./dist/plugin.cjs",
"default": "./dist/plugin.js"
},
"./pure": {
Expand Down
12 changes: 2 additions & 10 deletions playground/multi-exports-commonjs/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,15 @@
".": {
"types": "./dist/index.d.ts",
"source": "./src/index.ts",
"require": "./dist/index.js",
"node": {
"module": "./dist/index.esm.js",
"import": "./dist/index.cjs.mjs"
},
"import": "./dist/index.esm.js",
"require": "./dist/index.js",
"default": "./dist/index.esm.js"
},
"./plugin": {
"types": "./dist/plugin.d.ts",
"source": "./src/plugin.ts",
"require": "./dist/plugin.js",
"node": {
"module": "./dist/plugin.esm.js",
"import": "./dist/plugin.cjs.mjs"
},
"import": "./dist/plugin.esm.js",
"require": "./dist/plugin.js",
"default": "./dist/plugin.esm.js"
},
"./package.json": "./package.json"
Expand Down
6 changes: 1 addition & 5 deletions playground/ts-bundler/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,8 @@
".": {
"types": "./dist/index.d.ts",
"source": "./src/index.ts",
"require": "./dist/index.cjs",
"node": {
"module": "./dist/index.js",
"import": "./dist/index.cjs.js"
},
"import": "./dist/index.js",
"require": "./dist/index.cjs",
"default": "./dist/index.js"
},
"./package.json": "./package.json"
Expand Down
6 changes: 1 addition & 5 deletions playground/ts-node16/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,8 @@
".": {
"types": "./dist/index.d.ts",
"source": "./src/index.ts",
"require": "./dist/index.cjs",
"node": {
"module": "./dist/index.js",
"import": "./dist/index.cjs.js"
},
"import": "./dist/index.js",
"require": "./dist/index.cjs",
"default": "./dist/index.js"
},
"./package.json": "./package.json"
Expand Down
6 changes: 1 addition & 5 deletions playground/ts/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,8 @@
".": {
"types": "./dist/index.d.ts",
"source": "./src/index.ts",
"require": "./dist/index.cjs",
"node": {
"module": "./dist/index.js",
"import": "./dist/index.cjs.js"
},
"import": "./dist/index.js",
"require": "./dist/index.cjs",
"default": "./dist/index.js"
},
"./package.json": "./package.json"
Expand Down
47 changes: 13 additions & 34 deletions src/node/core/pkg/loadPkgWithReporting.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,52 +33,37 @@ export async function loadPkgWithReporting(options: {
logger.error(`exports["${expPath}"]: the \`types\` property should be the first property`)
}

if (exp.node) {
const nodeKeys = Object.keys(exp.node)

if (!assertOrder('module', 'import', nodeKeys)) {
shouldError = true
logger.error(
`exports["${expPath}"]: the \`node.module\` property should come before the \`node.import\` property`,
)
}

if (!assertOrder('import', 'require', nodeKeys)) {
logger.warn(
`exports["${expPath}"]: the \`node.import\` property should come before the \`node.require\` property`,
)
}

if (!assertOrder('module', 'require', nodeKeys)) {
logger.warn(
`exports["${expPath}"]: the \`node.module\` property should come before \`node.require\` property`,
)
}
if (exp.module) {
shouldError = true
logger.error(
`exports["${expPath}"]: the \`module\` condition shouldn't be used as it's not well supported in all bundlers.`,
)
}

if (exp.node) {
if (exp.import && exp.node.import && !assertOrder('node', 'import', keys)) {
shouldError = true
logger.error(
`exports["${expPath}"]: the \`node\` property should come before the \`import\` property`,
)
}

if (exp.module && exp.node.module && !assertOrder('node', 'module', keys)) {
if (exp.node.module) {
shouldError = true
logger.error(
`exports["${expPath}"]: the \`node\` property should come before the \`module\` property`,
`exports["${expPath}"]: the \`node.module\` condition shouldn't be used as it's not well supported in all bundlers. A better strategy is to refactor the codebase to no longer be vulnerable to the "dual package hazard"`,
)
}

if (
!exp.node.source &&
exp.node.import &&
!exp.node.require &&
exp.node.module &&
exp.import &&
exp.node.module !== exp.import
(exp.node.require || exp.require) &&
(exp.node.import.endsWith('.cjs.js') || exp.node.import.endsWith('.cjs.mjs'))
) {
shouldError = true
logger.error(
`exports["${expPath}"]: the \`node.module\` property should match \`import\``,
`exports["${expPath}"]: the \`node.import\` re-export pattern shouldn't be used as it's not well supported in all bundlers. A better strategy is to refactor the codebase to no longer be vulnerable to the "dual package hazard"`,
)
}

Expand All @@ -99,12 +84,6 @@ export async function loadPkgWithReporting(options: {
`exports["${expPath}"]: the \`import\` property should come before the \`require\` property`,
)
}

if (!assertOrder('module', 'import', keys)) {
logger.warn(
`exports["${expPath}"]: the \`module\` property should come before \`import\` property`,
)
}
}

if (!assertLast('default', keys)) {
Expand Down
2 changes: 0 additions & 2 deletions src/node/core/pkg/validatePkg.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,10 @@ const pkgSchema = z.object({
node: z.optional(
z.object({
source: z.optional(z.string()),
module: z.optional(z.string()),
import: z.optional(z.string()),
require: z.optional(z.string()),
}),
),
module: z.optional(z.string()),
import: z.optional(z.string()),
require: z.optional(z.string()),
default: z.string(),
Expand Down
46 changes: 20 additions & 26 deletions test/parseExports.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,17 +192,6 @@ describe('parseExports', () => {
},
`"node" can output both a "import" and "require" condition`,
],
[
{
require: './dist/index.cjs',
node: {
module: './dist/index.js',
import: './dist/index.cjs.js',
},
import: './dist/index.js',
},
`"node" can be used to implement a wrapper that protects against the "dual package hazard"`,
],
])('%o', (json, msg) => {
const extMap = getPkgExtMap({legacyExports: false})

Expand Down Expand Up @@ -272,50 +261,55 @@ describe('parseExports', () => {
import: './dist/index.js',
node: {
module: './dist/index.js',
import: './dist/index.cjs.js',
},
default: './dist/index.js',
},
`"node.import" must be before "import"`,
`"node.module" considered harmful`,
],
[
{
source: './src/index.ts',
node: {
module: './dist/index.js',
import: './dist/index.cjs.js',
require: './dist/index.cjs',
},
import: './dist/index.js',
module: './dist/index.js',
require: './dist/index.cjs',
import: './dist/index.js',
default: './dist/index.js',
},
`"node.require" is unnecesary if it's the same as "require"`,
`"module" considered harmful`,
],
[
{
source: './src/index.ts',
require: './dist/index.cjs',
import: './dist/index.js',
node: {
import: './dist/index.cjs.js',
import: './dist/index.node.js',
},
import: './dist/index.js',
default: './dist/index.js',
},
`"node.module" should be specified when a "node.import" dual package hazard is used`,
`"node.import" must be before "import"`,
],
[
{
source: './src/index.ts',
node: {
import: './dist/index.node.js',
require: './dist/index.cjs',
},
import: './dist/index.js',
require: './dist/index.cjs',
default: './dist/index.js',
},
`"node.require" is unnecesary if it's the same as "require"`,
],
[
{
require: './dist/index.cjs',
module: './dist/index.js',
node: {
import: './dist/index.cjs.js',
},
import: './dist/index.js',
default: './dist/index.js',
},
`"module" should be moved to "node.module" when it's the same as "import"`,
`the "node" re-export pattern considered harmful, protect against the "dual package hazard" in ways that have high ecosystem compatibility`,
],
[
{
Expand Down

0 comments on commit d481e4a

Please sign in to comment.