Skip to content

Move @types/* to devDependencies - #295

Closed
Kakulukian wants to merge 1 commit into
node-saml:masterfrom
Kakulukian:master
Closed

Move @types/* to devDependencies#295
Kakulukian wants to merge 1 commit into
node-saml:masterfrom
Kakulukian:master

Conversation

@Kakulukian

Copy link
Copy Markdown

Description

This PR updates the package.json file for the project to move the @types/ packages from dependencies to devDependencies. I have custom typing for @types/passport, and this package will overwrite it

@srd90

srd90 commented Jul 7, 2023

Copy link
Copy Markdown

Its not so long ago that @node-saml/node-saml and @node-saml/passport-saml projects moved types from devDependencies to dependencies. Rationale for that can be read from these PRs:

@Kakulukian your PR would take opposite direction and it seems that your direction would be against Typescript recommendations.

@srd90

srd90 commented Jul 7, 2023

Copy link
Copy Markdown

@cjbarth I wonder why @types/passport is listed at dependencies at all (normal or sev) because @node-saml/node-saml was supposed to be library without dependencies to any specific authentication frameworks / JS web frameworks. I.e. it was supposed to contain only SAML message processing / generation code utilized by e.g. @node-saml/passport-saml.

Maybe this PR could be changed so that @types/passport is removed and those types which were earlier moved from devDependencies to dependencies (by earlier mentioned PR) stay there (instead of moving all of them back to devDependencies)?

@cjbarth

cjbarth commented Jul 13, 2023

Copy link
Copy Markdown
Collaborator

Superseded by #296

@cjbarth cjbarth closed this Jul 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants