Skip to content
This repository has been archived by the owner on Jun 11, 2020. It is now read-only.

Add @babel/* to dependenciesWhitelist.txt #489

Merged
2 commits merged into from Oct 2, 2018

Conversation

mgroenhoff
Copy link
Contributor

No description provided.

@ghost
Copy link

ghost commented Oct 1, 2018

Could you link to the DefinitelyTyped PR that needs this?

@43081j
Copy link
Contributor

43081j commented Oct 1, 2018

babel are accepting PRs to add types to their own repo now, i think introducing any extra types to DT is the wrong direction to be heading in...

babel/babel#8629
babel/babel#8628
babel/babel#8627
babel/babel#8170
etc

Basically all @babel/* packages should be deprecated in the near future and users should instead use babel's own definitions.

@mgroenhoff
Copy link
Contributor Author

I did not yet create a PR but after this is merged I will. I am also not introducing those types. They already exist. This is required in order to remove them according to the instructions. Currently @babel/types and @babel/parser are the only packages with types that are published. I expect the others to eventually have them as well.

@43081j
Copy link
Contributor

43081j commented Oct 1, 2018

can you elaborate on the need for this in order to remove them? not disagreeing but rather interested as to what the plan is here.

the aim is to get every babel package to ship its own types and have a deprecation notice if anyone tries to use the DT babel types, if possible.

@mgroenhoff
Copy link
Contributor Author

I followed the procedure at https://github.com/DefinitelyTyped/DefinitelyTyped#removing-a-package to remove the types for @babel/parser and it told me that it needs to be added to this file first so that is what I did.

@mgroenhoff
Copy link
Contributor Author

If I remember correctly this was needed because I needed to add @babel/parser as a dependency to @babel/core

@43081j
Copy link
Contributor

43081j commented Oct 1, 2018

ah now i understand. you are removing @babel/parser but in order to do that, you need to add it to the whitelist so other babel packages can reference the shipped one.

is it possible we could remove all @babel/ packages from DT in one PR? to avoid having to add them each to this whitelist per removal? how many are currently in DT, if you know?

edit: could be a dumb suggestion if there's a few babel packages. but if there's only a couple it may be worth it

@mgroenhoff
Copy link
Contributor Author

mgroenhoff commented Oct 1, 2018

@43081j Exactly.

I think not at this moment because only @babel/types and @babel/parser have their types published. I can however add all @babel to the whitelist.

@43081j
Copy link
Contributor

43081j commented Oct 1, 2018

It looks like there's quite a few babel packages in here:

  • @babel/code-frame
  • babel-code-frame
  • @babel/core
  • babel-core
  • @babel/generator
  • babel-generator
  • @babel/parser
  • @babel/traverse
  • babel-traverse
  • babel-types

So you are indeed correct. we need to add the parser to the whitelist, remove it and update all the other packages to use it.

Until there's some activity in the babel repo (they are slow to sort the PRs out it seems), we'll have to do this for each babel package by the looks of it.

We can't remove all the babel-* ones either yet as they live in their own little world (not using any of babel's shipped types and not compatible with them either). once we have all babel types in the babel repo, all of those will have to go at once i imagine.

@ghost
Copy link

ghost commented Oct 1, 2018

@43081j Want to update this PR to include all the packages you'll need?

@43081j
Copy link
Contributor

43081j commented Oct 1, 2018

@mgroenhoff is creator FYI.

If he can add them, the ones we will need are:

  • @babel/code-frame
  • @babel/core
  • @babel/generator
  • @babel/parser
  • @babel/traverse

as far as i can see. the old-old types (babel-*) will just need to be removed in one jumbo PR it seems, once babel ships the remaining types.

we will just have to remember to remove these from the whitelist once the associated DT types are removed.

@mgroenhoff
Copy link
Contributor Author

@43081j I dont think removing babel-* is a good idea because they correspond to babel v6 and @babel/* correspond to babel v7.

@andy-ms Now that I got your attention 😄 Can you take a look at DefinitelyTyped/DefinitelyTyped#27729?

@mgroenhoff mgroenhoff changed the title Add @babel/parser to dependenciesWhitelist.txt Add @babel/* to dependenciesWhitelist.txt Oct 2, 2018
@43081j
Copy link
Contributor

43081j commented Oct 2, 2018

that's a fair point.

PR looks good to me too now. gives us chance to remove one at a time 👍

@ghost ghost merged commit 26b1168 into microsoft:master Oct 2, 2018
@mgroenhoff mgroenhoff deleted the @babel/parser branch October 3, 2018 08:33
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants