pnp / pnpjs

Fluent JavaScript API for SharePoint and Microsoft Graph REST APIs
https://pnp.github.io/pnpjs/
Other
763 stars 304 forks source link

Added Support to Typescript moduleResolution node16 and higher #2863

Closed trey-trimble-posh closed 9 months ago

trey-trimble-posh commented 10 months ago

Target environment

NodeJS

Additional environment details

Using Node 16 with typescript. Node type is commonjs with tsconfig

{
  "compilerOptions": {
    "target": "ES6",
    "module": "NodeNext",
    "moduleResolution": "NodeNext",
    "outDir": "./dist",
    "resolveJsonModule": true,
    "strictNullChecks": false,
    "strict": true,
    "esModuleInterop": true
  },
  "include": ["src/**/*", "tests/**/*", "types/**/*"],
  "ts-node": {
    "files": true
  }
}

Enhancement Idea

We are currently running a commonjs node application. Using node16 we can use @pnp/sp with the following tsconfig options:

{
    "target": "ES6",
    "module": "NodeNext",
    "moduleResolution": "NodeNext", 
}

However because of Node 16's restriction for moduleResolution on require file extension on relative imports the type augmentation fails for the following. I'm using all of this in a .mts file. Can't tell if this will become supported at some point either.

import "@pnp/sp/folders/index.js";
import "@pnp/sp/items/index.js";
import "@pnp/sp/lists/index.js";
import "@pnp/sp/webs/index.js";

import { Configuration } from "@azure/msal-node";
import { SPDefault } from "@pnp/nodejs";
import { spfi, SPFI } from "@pnp/sp/index.js";

This leads to a bunch of ugly @ts-expect-error messages. Simple fix is to supply a file extension in the module files. from

import { Web } from "./types.js";
export { IWeb, IWebs, Web, IWebAddResult, IWebUpdateResult, Webs, IWebInfo, IStorageEntity, IWebInfosData, } from "./types.js";
declare module "../fi" {
    interface SPFI {
        /**
         * Access to the current web instance
         */
        readonly web: ReturnType<typeof Web>;
    }
}
//# sourceMappingURL=index.d.ts.map

to

import { Web } from "./types.js";
export { IWeb, IWebs, Web, IWebAddResult, IWebUpdateResult, Webs, IWebInfo, IStorageEntity, IWebInfosData, } from "./types.js";
declare module "../fi.js" {
    interface SPFI {
        /**
         * Access to the current web instance
         */
        readonly web: ReturnType<typeof Web>;
    }
}
//# sourceMappingURL=index.d.ts.map
juliemturner commented 10 months ago

Yes, we're aware and it's on our V4 backlog here: https://github.com/orgs/pnp/projects/1/views/1

patrick-rodgers commented 9 months ago

For this item we've explored including both commonjs and esm in the module package, but it caused more problems than it solved. The situation you describe is really a result of how the node commonjs, typescript, and esm module resolution story is all a bit of a mess vs an issue with this library. As the fix is simply to include the file extension its a case where trying to solve it is worse than the initial problem. For v4 we will continue with publishing esm modules. But we did do a significant investigation, so don't want you to think we didn't look into things. Thanks!

github-actions[bot] commented 9 months ago

This issue is locked for inactivity or age. If you have a related issue please open a new issue and reference this one. Closed issues are not tracked.