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

Build to dist folder for publishing as an npm module #783

Closed
wants to merge 1 commit into from

Conversation

nfadili
Copy link

@nfadili nfadili commented Oct 29, 2023

Summary

This is a follow up to my comment here #763 (comment)

There's various ways this could be configured but I opted to go with this simple set of changes just so I could get the idea across and hopefully discuss more with you. These are the steps I took to verify the changes:

  1. npm run compile-lib - generates a dist folder of the usable javascript and their associated types in .d.ts files
  2. npm pack - packs the npm module - the final module looks like:
npm notice 📦  [email protected]
npm notice === Tarball Contents === 
npm notice 16.6kB README.md                  
npm notice 760B   dist/digest-auth.d.ts      
npm notice 3.4kB  dist/digest-auth.js        
npm notice 11.1kB dist/musicbrainz-api.d.ts  
npm notice 20.6kB dist/musicbrainz-api.js    
npm notice 15.4kB dist/musicbrainz.types.d.ts
npm notice 873B   dist/musicbrainz.types.js  
npm notice 230B   dist/rate-limiter.d.ts     
npm notice 1.1kB  dist/rate-limiter.js       
npm notice 353B   dist/xml/xml-isrc-list.d.ts
npm notice 587B   dist/xml/xml-isrc-list.js  
npm notice 178B   dist/xml/xml-isrc.d.ts     
npm notice 343B   dist/xml/xml-isrc.js       
npm notice 183B   dist/xml/xml-metadata.d.ts 
npm notice 969B   dist/xml/xml-metadata.js   
npm notice 526B   dist/xml/xml-recording.d.ts
npm notice 518B   dist/xml/xml-recording.js  
npm notice 2.8kB  package.json               
npm notice === Tarball Details === 
npm notice name:          musicbrainz-api                         
npm notice version:       0.10.3                                  
npm notice filename:      musicbrainz-api-0.10.3.tgz              
npm notice package size:  17.3 kB                                 
npm notice unpacked size: 76.6 kB                                 
npm notice shasum:        c77db44a30807d83854089333429a4c9594a0f36
npm notice integrity:     sha512-gZQv6dde8PtRt[...]VLqksX0BiglUA==
npm notice total files:   18

Follow ups

  • I disabled source maps for lib builds because they don't have a purpose in the npm module that I can tell? I imagine you use them for deving, so that might take some reconfiguring.
  • I'm not sure what your process is for publishing to npm so I just made some assumptions 😄
  • If you decide to move forward with this, versioning would take some consideration. Not entirely sure if this would be a breaking change but I suspect it might be.

Copy link
Owner

@Borewit Borewit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disabled source maps for lib builds because they don't have a purpose in the npm module that I can tell? I imagine you use them for deving, so that might take some reconfiguring.

That is indeed why I have those enabled. Users also complain on my other modules, that it causing some problems.

Hi @nfadili, I indeed use the source map during development. I can try without

@@ -1,7 +1,8 @@
{
"extends": "../tsconfig.json",
"compilerOptions": {
"sourceMap": true,
"outDir": "../dist",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had problems with IDE in the past separating the source and build directories. Is this required? Source maps are already excluded from the distribution here:

lib/**/*.js.map

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By specifying an explicit distribution folder, you can more easily control what gets shipped to NPM for download by other projects. Your current configuration outputs built files (.js and .d.ts) alongside your source code (.ts files) and you ship all of it to NPM. By building to the dist folder and shipping just that, consumers of the package get just the runnable JS code and optional types.

As for source maps and the dev workflow: the way I usually approach this is by separating my build and dev workflows. I have a set of commands (typically npm scripts) that I use for deving that have source maps and whatever else might make my life easier. And then I have a set of commands for building a distribution files. You can even have a separate tsconfig.json for each if you want to control all aspects of it

@nfadili nfadili mentioned this pull request Nov 7, 2023
@nfadili nfadili closed this Nov 7, 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.

2 participants