Skip to content

Commit

Permalink
Fix import errors from core package once again
Browse files Browse the repository at this point in the history
After restructuring the core package, dynamic imports did not work any longer because apparently, they must not be called from submodules. Relates #65 and microsoft/TypeScript#43329, again.
  • Loading branch information
LinqLover committed Oct 12, 2021
1 parent 26205b1 commit 0a4cf73
Show file tree
Hide file tree
Showing 7 changed files with 82 additions and 58 deletions.
2 changes: 2 additions & 0 deletions packages/cli/src/oclif.init.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import dotenv from 'dotenv'
import { Hook } from '@oclif/config'
import { loadExternalModules } from 'dowdep'

export const hook: Hook<'init'> = async function (options) {
dotenv.config()
await loadExternalModules()
}
7 changes: 2 additions & 5 deletions packages/core/src/dependencies/sourcegraph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import normalizePackageData from 'normalize-package-data'

import { Dependency, DependencySearcher } from './base'
import { Dowdep } from '../dowdep'
import externalModules from '../externalModules'
import { Package } from '../packages'
import { OnlyData } from '../utils/OnlyData'

Expand Down Expand Up @@ -147,11 +148,7 @@ class SourcegraphClient {
'count': `${limit || this.maximumLimit}`
}

// Workaround for https://github.com/microsoft/vscode/issues/130367 and https://github.com/microsoft/TypeScript/issues/43329 🤯
const dynamicImport = new Function('moduleName', 'return import(moduleName)')
const escapeRegexp: (regex: string) => string = (await dynamicImport('escape-string-regexp')).default

const query = `"${escapeRegexp(packageName)}": ` + Object.entries(queryArgs).map(([key, value]) => `${key}:${value}`).join(' ')
const query = `"${externalModules.escapeRegexp(packageName)}": ` + Object.entries(queryArgs).map(([key, value]) => `${key}:${value}`).join(' ')
const response = this.documentSpecifier.protoResponse
Object.assign(response, await graphql.request(this.documentSpecifier.document, { query }))

Expand Down
19 changes: 19 additions & 0 deletions packages/core/src/externalModules.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import type { Import, Options } from "parse-imports"


/**
* Stores external modules that have been loaded in a non-standard, weird way.
*
* The existence of this is a large workaround. Some components of our toolchain, including oclif and jest, do not support dynamic importing of pure ES modules correctly. See `loadExternalModules()` in `index.ts`. As the technique used there cannot be applied from submodules, imported modules are stored in this file.
*/
const modules = {
/** Escape RegExp special characters */
escapeRegexp: <(regex: string) => string><any>undefined,
/** A blazing fast ES module imports parser. */
parseImports: <(
code: string,
options?: Options
) => Promise<Iterable<Import>>><any>undefined
}

export default modules
49 changes: 49 additions & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,52 @@ export {
ReferenceSearcher,
ReferenceSearchStrategy
} from './references'

import pkgDir from 'pkg-dir'
import externalModules from './externalModules'

let _loadedExternalModules = false

/** Load external modules asynchronously that cannot be imported in the usual way. See `externalModules.ts` for details. This function must be called before using any other functionality of this package! */
export async function loadExternalModules() {
if (_loadedExternalModules) {
return
}

/**
* Truly awful hack! There are a few things going on here:
* - Jest (or something) can't find parse-imports by just importing its package name
* no matter what. Just give it the path to the src/index.js file
* - Workaround for https://github.com/microsoft/TypeScript/issues/43329.
*
* - TypeScript will always try to replace dynamic imports with `requires` which doesn't work for importing ESM from CJS.
* We work around by "hiding" our dynamic import in a Function constructor (terrible...).
*
* In particular, we must not extract this call into a separate module.
* This would result in sporadic unresolved promises in the jest environment.
* See #65.
*
* All of this required jest@next, ts-jest@next, AND `NODE_OPTIONS=--experimental-vm-modules`
*
* Developed with the kind help of @TomerAbach.
*/
const dynamicImport = new Function('moduleName', 'return import(moduleName)')

externalModules.escapeRegexp = (await dynamicImport('escape-string-regexp')).default
const parseImportsIndexPath = `${await pkgDir()}/node_modules/parse-imports/src/index.js`

try {
externalModules.parseImports = (await dynamicImport(parseImportsIndexPath)).default
} catch (parseError) {
if (!(parseError instanceof Error && 'code' in parseError && (<{ code: string }>parseError).code == 'ERR_MODULE_NOT_FOUND')) {
throw parseError
}
// This will occur if this package is imported as a local dependency from another package via a symlink.
// For now, let's handle this by assuming the depending package is a sibling of ourselves ...
// Hardcoded! So many hacks! 😭
const parseImportsIndexPath = `${await pkgDir()}/../core/node_modules/parse-imports/src/index.js`
externalModules.parseImports = (await dynamicImport(parseImportsIndexPath)).default
}

_loadedExternalModules = true
}
53 changes: 3 additions & 50 deletions packages/core/src/references/heuristic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import path from 'path'
import pkgDir from 'pkg-dir'

import { DeclarationImport, FilePosition, ReferenceSearcher, Reference, ReferenceLocation } from './base'
import externalModules from '../externalModules'
import rex from '../utils/rex'


Expand Down Expand Up @@ -40,25 +41,12 @@ export class HeuristicReferenceSearcher extends ReferenceSearcher {

const identifierPattern = /[\p{L}\p{Nl}$_][\p{L}\p{Nl}$\p{Mn}\p{Mc}\p{Nd}\p{Pc}]*/u

/**
* Workaround for https://github.com/microsoft/TypeScript/issues/43329.
*
* TypeScript will always try to replace dynamic imports with `requires` which doesn't work for importing ESM from CJS.
* We work around by "hiding" our dynamic import in a Function constructor (terrible...).
*
* In particular, we must not extract this call into a separate module.
* This would result in sporadic unresolved promises in the jest environment.
* See #65.
*/
const dynamicImport = new Function('moduleName', 'return import(moduleName)')
const escapeRegexp: (regex: string) => string = (await dynamicImport('escape-string-regexp')).default

const requirePattern = rex`
(?<alias> ${identifierPattern} ) \s*
= \s*
require \s* \( \s*
(?<quote>['"])
(?<packageName> ${escapeRegexp(this.$package.name)} )
(?<packageName> ${externalModules.escapeRegexp!(this.$package.name)} )
(
\/ (?<memberName> ${identifierPattern} )
)?
Expand Down Expand Up @@ -154,42 +142,7 @@ export class HeuristicReferenceSearcher extends ReferenceSearcher {
async* collectEsmBindings(source: string): AsyncGenerator<ModuleBinding, void, undefined> {
const imports = await (async () => {
try {
// Truly awful hack! There are a few things going on here:
// - Jest (or something) can't find parse-imports by just importing its package name
// no matter what. Just give it the path to the src/index.js file
//
// - Workaround for https://github.com/microsoft/TypeScript/issues/43329.
//
// TypeScript will always try to replace dynamic imports with `requires` which doesn't work for importing ESM from CJS.
// We work around by "hiding" our dynamic import in a Function constructor (terrible...).
//
// In particular, we must not extract this call into a separate module.
// This would result in sporadic unresolved promises in the jest environment.
// See #65.
//
// - All of this required jest@next, ts-jest@next, AND `NODE_OPTIONS=--experimental-vm-modules`
const parseImportsIndexPath = `${await pkgDir()}/node_modules/parse-imports/src/index.js`
const dynamicImport = new Function('moduleName', 'return import(moduleName)')
let parseImports: (
code: string,
options?: Options
) => Promise<Iterable<Import>>

try {
parseImports = (await dynamicImport(parseImportsIndexPath)).default
} catch (parseError) {
if (!(parseError instanceof Error && 'code' in parseError && (<{ code: string }>parseError).code == 'ERR_MODULE_NOT_FOUND')) {
throw parseError
}
// This will occur if this package is imported as a local dependency from another package via a symlink.
// For now, let's handle this by assuming the depending package is a sibling of ourselves ...
// Hardcoded! So many hacks! 😭
const parseImportsIndexPath = `${await pkgDir()}/../core/node_modules/parse-imports/src/index.js`
const dynamicImport = new Function('moduleName', 'return import(moduleName)')
parseImports = (await dynamicImport(parseImportsIndexPath)).default
}

return await parseImports(source)
return await externalModules.parseImports(source)
} catch (parseError) {
console.warn("Error from parse-imports", { parseError, source: source.slice(0, 100), dependencyName: this.dependency.name }) // TODO: Make getter denedencyName?
// This includes syntax errors but also TypeScript syntax which is not (yet?) supported by parse-imports.
Expand Down
3 changes: 3 additions & 0 deletions packages/core/test/references.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import ifCurtailed from '../src/utils/if-curtailed'
import { lodashClassifyNested } from '../src/utils/lodash-classify'
import { getSourceDirectory } from './_utils/sourceDirectory'
import { printDiff } from './_utils/printDiff'
import { loadExternalModules } from '../src'


const SOURCE_DIRECTORY = getSourceDirectory(__filename)
Expand Down Expand Up @@ -37,6 +38,8 @@ describe('ReferenceSearcher', () => {
([packageName, expectedReferences]) => ({ strategy: <ReferenceSearchStrategy>strategy, packageName, expectedReferences }))
))("should find relevant references for %s", async (
{ strategy, packageName, expectedReferences }) => {
await loadExternalModules()

const $package = new Package(
packageName,
`${SOURCE_DIRECTORY}/examples/packages/${packageName}`
Expand Down
7 changes: 4 additions & 3 deletions packages/vscode-extension/src/extension.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { DeclarationLocation, Dependency, Dowdep, FilePosition, Package, Reference, ReferenceSearchStrategy } from 'dowdep'
import { DeclarationLocation, Dependency, Dowdep, FilePosition, loadExternalModules, Package, Reference, ReferenceSearchStrategy } from 'dowdep'
import _ from 'lodash'
import filterAsync from 'node-filter-async'
import normalizePackageData from 'normalize-package-data'
Expand All @@ -23,12 +23,13 @@ type PackageLike = Package | readonly Package[]
/**
* This method is called on the first activation event.
*/
export function activate(context: vscode.ExtensionContext) {
console.log("The extension \"dowdep\" is now active.")
export async function activate(context: vscode.ExtensionContext) {
await loadExternalModules()

extension = new Extension(context)
extension.activate()

console.log("The extension \"dowdep\" is now active.")
return extension
}

Expand Down

0 comments on commit 0a4cf73

Please sign in to comment.