Pnpjs: PnPJS typings referencing .js files

Created on 19 Jan 2021  路  12Comments  路  Source: pnp/pnpjs

Category

  • [ ] Enhancement
  • [x] Bug
  • [ ] Question
  • [ ] Documentation gap/issue

Version

Please specify what version of the library you are using: [ 2.1.0 ]

Please specify what version(s) of SharePoint you are targeting: [online]

Having some issues with PnPjs definitions (d.ts files), I can see that there are references to .js files, this must be wrong?

Screenshot 2021-01-19 at 12 20 30

package details needed discussion

Most helpful comment

I would add make the project happy. We have lots of various pipelines (F5 debug, build, test, serve, etc.) that all need to work. Making all that work in node 15 (and through that work webpack 5) was not trivial. We need to avoid doing something that makes all that overly complicated, slow, or introduce failures. Having to post-process all the files just to hit F5 debug is really undesirable for the folks working on the library. There is also an open issue on (#1438) on packaging just the types, so perhaps this work can feed into that work

Right now the first two items should be done (not that there won't be issues, but things are working generally). The third, definitions, are happy except in the case where someone copies them away from the source which introduces file not found issues due to the extension.

All 12 comments

Hmm. for some reason all the .ts code files are importing files at .js files. There must be reason for this, but I think these should be removed from the definitions. d.ts files?

image

This change was made to support Node 15 and Webpack 5 so unfortunately, we cannot change this...

@patrick-rodgers -- can you confirm about the .d.ts files?

The ".d.ts" files just take the literal strings we use for imports so this isn't a setting we can change. My understanding is that this doesn't affect the .d.ts files working as expected - are we seeing issues?

Ok, I got issue in my project, but got workaround for that. Im using the d.ts files as definitions in SP Editor, so with .js endings things will fail, but as I said, I fixed it in my project. Someone could have similar issue thou.

When I tested in SPFx project it all seemed to work. If you can provide a repro we can have a look. Certainly don't want to be breaking folks 馃檪

I guess it was/is in SP Editor?

I copy all d.ts files from node_modules/@pnp/* and load them to monaco-editor to have intelligence, not actually building anything. And as i don't copy the .js files, I get file not found kinda issues. But not a biggie for my project, as I'm already digging trough all the files so I can fix it just by modifying them. But anyways, d.ts files stand for definitions and do not really go with referencing .js/.ts files, or at least it should not work like that.

@tavikukko - understood and I agree. The entire module stuff is a bit of a mess currently and everyone is essentially saying it is the other folks problem. Will do some research on the best way to resolve this. Trying hard to not post-process all the files, but may come to that.

Yeah, maybe we just need a list of requirements and we can all try to think solution for this.

  • make node happy
  • make browser happy
  • make definitions happy
  • what else?

I would add make the project happy. We have lots of various pipelines (F5 debug, build, test, serve, etc.) that all need to work. Making all that work in node 15 (and through that work webpack 5) was not trivial. We need to avoid doing something that makes all that overly complicated, slow, or introduce failures. Having to post-process all the files just to hit F5 debug is really undesirable for the folks working on the library. There is also an open issue on (#1438) on packaging just the types, so perhaps this work can feed into that work

Right now the first two items should be done (not that there won't be issues, but things are working generally). The third, definitions, are happy except in the case where someone copies them away from the source which introduces file not found issues due to the extension.

Just dropping a link to a representative issue in the TS repo to inform this conversation. Long story short the TS answer is to include the ".js" extension in your code. TS handles resolving that during build.

There is also an open issue that seems to be around the declaration issue you bring up.

Was this page helpful?
0 / 5 - 0 ratings