Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] Allow typings to be set using publishConfig #137

Closed
1 of 2 tasks
bgotink opened this issue May 8, 2019 · 7 comments
Closed
1 of 2 tasks

[Feature] Allow typings to be set using publishConfig #137

bgotink opened this issue May 8, 2019 · 7 comments
Labels
enhancement New feature or request in progress Features claimed by a contributor

Comments

@bgotink
Copy link
Sponsor Member

bgotink commented May 8, 2019

  • I'd be willing to implement this feature
  • This feature can already be implemented through a plugin

Describe the user story

My typescript project uses Microsoft's API extractor, which allows filtering my APIs and which rolls up all .d.ts files into one. Some of my APIs are internal and marked as such via @internal in the tsdoc.

I want the packages in my workspace to be able to access them, but when published these APIs should be removed from the package's typings.

To go one step further: the API extractor allows marking APIs unstable (e.g. @beta), and supports filtering out all internal, alpha or beta packages. For instance, if I publish a beta version of my package, I'd like to keep the @beta APIs but remove the @alpha and @internal APIs.

Describe the solution you'd like

By allowing the typings property to be overridden in publishConfig, I can provide access to the unfiltered rolled up .d.ts file for the workspace packages while removing internal APIs in the published packages.

Describe the drawbacks of your solution

  • This is potentially a footgun, but no more than modifying main and module via publishConfig

Describe alternatives you've considered

  • I could run API extractor filtering out the internal properties in the prepublishOnly lifecycle script. That would break the build of all internal packages that consume my internal APIs. If we want to support publishing multiple packages at once this won't work.
  • My current solution is prefixing all internal APIs with a letter from the Greek alphabet and forcing dependants to use a tslint rule that disallows importing anything with a greek letter. This obviously doesn't work well.
  • Set the main to the typescript file instead of the built javascript, that way the @internal APIs are available and the typings property in the package manifest can just point to the trimmed rolled up .d.ts file.
    Our CI/CD setup tests the packages before publishing. We want these tests to run the code as close to production as posssible, i.e. we want to test the built packages and not use the typescript source of our internal dependencies in the tests.
@bgotink bgotink added the enhancement New feature or request label May 8, 2019
@arcanis
Copy link
Member

arcanis commented May 8, 2019

Set the main to the typescript file instead of the built javascript, that way the @internal APIs are available and the typings property in the package manifest can just point to the trimmed rolled up .d.ts file.

I guess to truly be as close from metal as possible you could go one step further and publish all your packages to a fake registry (for example a Verdaccio instance). That would trigger the full pack pipeline, including publishConfig.main replacement.

Another alternative would be to make a hook (beforeWorkspacePacking) that would have the ability to modify the manifest at publish-time. This would actually be interesting, because I had to duplicate the main and module logic for both pack and publish (since each of them require to transform the manifest independently) - this would give us a way to avoid doing that.

@bgotink
Copy link
Sponsor Member Author

bgotink commented May 8, 2019

I guess to truly be as close from metal as possible you could go one step further and publish all your packages to a fake registry (for example a Verdaccio instance). That would trigger the full pack pipeline, including publishConfig.main replacement.

Fair point, but how would that work with our tests that are contained within the workspace?

Another alternative would be to make a hook (beforeWorkspacePacking) that would have the ability to modify the manifest at publish-time.

I was thinking along the same lines, because it doesn't make sense to have to duplicate this between pack and publish. I was also wondering whether this should be part of the core or a separate plugin altogether. I'm leaning towards a separate plugin, which could also do the main and module change.

@bgotink
Copy link
Sponsor Member Author

bgotink commented May 8, 2019

This hook would also be useful for #133.

@arcanis
Copy link
Member

arcanis commented May 8, 2019

(I don't understand why but my first post appears at the bottom of this thread which is pretty annoying. Github probably messed up a date somehow 😔)

Fair point, but how would that work with our tests that are contained within the workspace?

Solvable with a dedicated infra (you could run the tests from your workspaces that would access the dependencies from your remote registry), but probably a fair amount of work for a low impact 🤔

I was also wondering whether this should be part of the core or a separate plugin altogether. I'm leaning towards a separate plugin, which could also do the main and module change.

I think main and module might be things we would want to have in plugin-essentials (because they have a far-reaching impact, particularly main which is recognized by Node), and types could be defined by plugin-typescript. Does that sound reasonable?

@bgotink
Copy link
Sponsor Member Author

bgotink commented May 9, 2019

Looks like they fixed it. Yesterday when I added my comments the OP and your comment were shown as "commented 3 hours from now" so they definitely messed up the dates.

I think main and module might be things we would want to have in plugin-essentials (because they have a far-reaching impact, particularly main which is recognized by Node), and types could be defined by plugin-typescript. Does that sound reasonable?

Sounds reasonable!

@arcanis arcanis added the in progress Features claimed by a contributor label May 10, 2019
@arcanis
Copy link
Member

arcanis commented May 12, 2019

Fixed by #148 👍

@arcanis arcanis closed this as completed May 12, 2019
@zkochan
Copy link
Contributor

zkochan commented May 17, 2019

I'm going to implement this in pnpm as well. This is an awesome feature!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request in progress Features claimed by a contributor
Projects
None yet
Development

No branches or pull requests

3 participants