Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
75 changes: 72 additions & 3 deletions lib/get-exports.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@

const getEsmExports = require('./get-esm-exports.js')
const { parse: parseCjs } = require('cjs-module-lexer')
const { readFileSync } = require('fs')
const { readFileSync, existsSync } = require('fs')
const { builtinModules } = require('module')
const { fileURLToPath, pathToFileURL } = require('url')
const { dirname } = require('path')
const { dirname, join } = require('path')

function addDefault (arr) {
return new Set(['default', ...arr])
Expand All @@ -27,6 +27,64 @@

const urlsBeingProcessed = new Set() // Guard against circular imports.

/**
* This function looks for the package.json which contains the specifier trying to resolve.
* Once the package.json file has been found, we extract the file path from the specifier
* @param {*} specifier The specifier that is being search for inside the imports object
* @param {*} fromUrl The url from which the search starts from
* @returns file to url to file export
*/
function resolvePackageImports (specifier, fromUrl) {
if (!specifier.startsWith('#')) {
return null
}

try {
const fromPath = fileURLToPath(fromUrl)
let currentDir = dirname(fromPath)

// search for package.json file which has the real url to export
while (currentDir !== dirname(currentDir)) {
const packageJsonPath = join(currentDir, 'package.json')

if (existsSync(packageJsonPath)) {
const packageJson = JSON.parse(readFileSync(packageJsonPath, 'utf8'))

if (packageJson.imports && packageJson.imports[specifier]) {
const imports = packageJson.imports[specifier]

// Look for path inside packageJson
let resolvedPath
if (typeof imports == 'object') {

Check failure on line 58 in lib/get-exports.js

View workflow job for this annotation

GitHub Actions / lint

Expected '===' and instead saw '=='
Copy link
Contributor

Choose a reason for hiding this comment

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

A better check is Object.prototype.toString.call(imports) === '[object Object]'.

Copy link
Member

Choose a reason for hiding this comment

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

@jsumners-nr I am surprised by that suggestion (when I just looked at it, I wanted to recommend to check for typeof and not null). That should be much faster and should suffice for the overall check here, if I am not mistaken. Am I missing something / what would the benefit be for calling toString()?

Copy link
Author

Choose a reason for hiding this comment

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

From what I understand, checking with toString() ensures we don't allow values such as [ ], null or Date, which are also of typeof 'object'

Copy link
Member

Choose a reason for hiding this comment

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

I understand that it would not match these anymore. We just check for the property existence on these objects and they should never have the properties we are looking for. Due to that, I favor the faster check.

Copy link
Author

Choose a reason for hiding this comment

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

I agree, I'll add that into the latest changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

@BridgeAR the original check was a coercive check for an Object. Various things, e.g Array, would mistakenly satisfy that check.

Copy link
Author

Choose a reason for hiding this comment

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

@jsumners-nr I reinstated it to the original check after considering that the code only performs operations based on certain properties, which are checked for existence before being used. This allows us to maintain the most efficient check.

const requireSpecifier = imports.require
const importSpecifier = imports.import
// look for the possibility of require and import which is standard for CJS/ESM
if (requireSpecifier || importSpecifier) {
// trying to resolve based on order of importance
resolvedPath = requireSpecifier.node || requireSpecifier.default || importSpecifier.node || importSpecifier.default
} else if (imports.node || imports.default) {
resolvedPath = imports.node || imports.default
}
} else if (typeof imports == 'string') {

Check failure on line 68 in lib/get-exports.js

View workflow job for this annotation

GitHub Actions / lint

Expected '===' and instead saw '=='
resolvedPath = imports
}

if (existsSync(resolvedPath)) {
return resolvedPath
}
}
// return if we find a package.json but did not find an import
return null
}
currentDir = dirname(currentDir)
}
} catch (error) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
} catch (error) {
} catch {

Alternatively, we could also just add the cause

throw new Error(`Failed to find sub path export: ${specifier}`)
}

return null
}

async function getCjsExports (url, context, parentLoad, source) {
if (urlsBeingProcessed.has(url)) {
return []
Expand All @@ -46,8 +104,19 @@
if (re === '.') {
re = './'
}

let newUrl
// Entries in the import field should always start with #
if (re.startsWith('#')) {
re = resolvePackageImports(re, url)
if (!re) {
// Unable to resolve specifier import
return
}
}

// Resolve the re-exported module relative to the current module.
const newUrl = pathToFileURL(require.resolve(re, { paths: [dirname(fileURLToPath(url))] })).href
newUrl = pathToFileURL(require.resolve(re, { paths: [dirname(fileURLToPath(url))] })).href

Check failure on line 119 in lib/get-exports.js

View workflow job for this annotation

GitHub Actions / lint

'newUrl' is never reassigned. Use 'const' instead

if (newUrl.endsWith('.node') || newUrl.endsWith('.json')) {
return
Expand Down
17 changes: 17 additions & 0 deletions test/fixtures/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"name": "test-fixtures",
"imports": {
"#main-entry-point": {
"require": {
"node": "./something.js",
"default": "./something.js"
},
"import": {
"node":"./something.mjs",
"default": "./something.js"
}
},
"#main-entry-point-string" : "./something.js",
"#main-entry-point-external" : "some-external-cjs-module"
}
}
1 change: 1 addition & 0 deletions test/fixtures/specifier-external.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = { ...require('#main-entry-point-external') }
1 change: 1 addition & 0 deletions test/fixtures/specifier-string.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = { ...require('#main-entry-point-string') }
1 change: 1 addition & 0 deletions test/fixtures/specifier.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = { ...require('#main-entry-point') }
11 changes: 11 additions & 0 deletions test/hook/specifier-external-imports.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { foo } from '../fixtures/specifier-external.js'
import Hook from '../../index.js'
import { strictEqual } from 'assert'

Hook((exports, name) => {
if (name.endsWith('fixtures/specifier-external.js')) {
exports.foo = 'bar2'
}
})

strictEqual(foo, 'bar2')
11 changes: 11 additions & 0 deletions test/hook/specifier-imports.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { foo } from '../fixtures/specifier.js'
import Hook from '../../index.js'
import { strictEqual } from 'assert'

Hook((exports, name) => {
if (name.endsWith('fixtures/specifier.js')) {
exports.foo = 1
}
})

strictEqual(foo, 1)
11 changes: 11 additions & 0 deletions test/hook/specifier-string-imports.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { foo } from '../fixtures/specifier-string.js'
import Hook from '../../index.js'
import { strictEqual } from 'assert'

Hook((exports, name) => {
if (name.endsWith('fixtures/specifier-string.js')) {
exports.foo = 1
}
})

strictEqual(foo, 1)
Loading