Skip to content

fix: build types from source instead of generated files #40

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jul 28, 2021
Merged

fix: build types from source instead of generated files #40

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1bc4f3e
fix: build types from source instead of generated files
achingbrain Jul 16, 2021
95a1d23
chore: copy examples separately
achingbrain Jul 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ node_modules/
coverage/
build/
dist/
types/
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
"scripts": {
"build": "npm run build:js && npm run build:types",
"build:js": "ipjs build --tests --main && npm run build:copy",
"build:copy": "mkdir -p dist/examples/ && cp -a tsconfig.json *.js *.ts lib/ test/ dist/ && cp examples/*.* dist/examples/",
Copy link
Member Author

@achingbrain achingbrain Jul 16, 2021

Choose a reason for hiding this comment

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

The trailing slash meant that the contents of the test folder (etc) was copied into dist rather than the test folder itself, which I guess was not intentional? Removing it means you don't need the extra cp command to copy the examples folder afterwards.

Copy link
Member Author

@achingbrain achingbrain Jul 16, 2021

Choose a reason for hiding this comment

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

By which I mean the copied contents are getting flattened in the dist dir and files can overwrite each other if they happen to have the same name so I think this was not intended.

With trailing slash:

$ ls -l dist
total 472
-rw-r--r--   1 alex  staff    554 16 Jul 08:59 LICENSE-APACHE
-rw-r--r--   1 alex  staff   1053 16 Jul 08:59 LICENSE-MIT
-rw-r--r--   1 alex  staff  29679 16 Jul 08:59 README.md
-rw-r--r--   1 alex  staff   1258 16 Jul 07:10 api.ts
-rw-r--r--   1 alex  staff    395 26 May 10:52 car-browser.js
-rw-r--r--   1 alex  staff    371 26 May 10:52 car.js
drwxr-xr-x   7 alex  staff    224 16 Jul 08:59 cjs
-rw-r--r--   1 alex  staff    809 26 May 10:52 coding.ts
-rw-r--r--   1 alex  staff   8008 16 Jul 07:10 common.js
-rw-r--r--   1 alex  staff   8056 16 Jul 07:10 decoder.js
-rw-r--r--   1 alex  staff   1653 26 May 10:52 encoder.js
drwxr-xr-x   8 alex  staff    256 16 Jul 08:59 esm
drwxr-xr-x   9 alex  staff    288 16 Jul 08:59 examples
-rw-r--r--   1 alex  staff    715 26 May 10:52 go.car
-rw-r--r--   1 alex  staff     49 16 Jul 08:59 index.js
-rw-r--r--   1 alex  staff     64 16 Jul 08:59 indexed-reader
-rw-r--r--   1 alex  staff    153 26 May 10:52 indexed-reader-browser.js
-rw-r--r--   1 alex  staff   6203 16 Jul 07:10 indexed-reader.js
-rw-r--r--   1 alex  staff     49 16 Jul 08:59 indexer
-rw-r--r--   1 alex  staff   4120 26 May 10:52 indexer.js
-rw-r--r--   1 alex  staff     50 16 Jul 08:59 iterator
-rw-r--r--   1 alex  staff   1956 26 May 10:52 iterator-channel.js
-rw-r--r--   1 alex  staff   9000 26 May 10:52 iterator.js
-rw-r--r--   1 alex  staff   2311 26 May 10:52 node-test-file-streams.js
-rw-r--r--   1 alex  staff   1371 26 May 10:52 node-test-indexed-reader.js
-rw-r--r--   1 alex  staff   3786 26 May 10:52 node-test-large.js
-rw-r--r--   1 alex  staff   2466 26 May 10:52 node-test-raw.js
-rw-r--r--   1 alex  staff   2692 26 May 10:52 node-test-updateroots.js
-rw-r--r--   1 alex  staff   4517 16 Jul 08:59 package.json
-rw-r--r--   1 alex  staff     56 16 Jul 08:59 reader
-rw-r--r--   1 alex  staff   5767 16 Jul 07:10 reader-browser.js
-rw-r--r--   1 alex  staff   2069 16 Jul 07:10 reader.js
-rw-r--r--   1 alex  staff   2578 16 Jul 07:10 test-errors.js
-rw-r--r--   1 alex  staff   1966 26 May 10:52 test-indexer.js
-rw-r--r--   1 alex  staff    831 26 May 10:52 test-interface.js
-rw-r--r--   1 alex  staff   2555 26 May 10:52 test-iterator.js
-rw-r--r--   1 alex  staff   2982 26 May 10:52 test-reader.js
-rw-r--r--   1 alex  staff   9726 26 May 10:52 test-writer.js
-rw-r--r--   1 alex  staff   1252 16 Jul 07:36 tsconfig.json
drwxr-xr-x  10 alex  staff    320 16 Jul 08:59 types
-rw-r--r--   1 alex  staff   4863 26 May 10:52 verify-store-reader.js
-rw-r--r--   1 alex  staff     56 16 Jul 08:59 writer
-rw-r--r--   1 alex  staff   8018 16 Jul 07:10 writer-browser.js
-rw-r--r--   1 alex  staff   3253 26 May 10:52 writer.js

Without trailing slash:

$ ls -l dist
total 176
-rw-r--r--   1 alex  staff    554 16 Jul 08:59 LICENSE-APACHE
-rw-r--r--   1 alex  staff   1053 16 Jul 08:59 LICENSE-MIT
-rw-r--r--   1 alex  staff  29679 16 Jul 08:59 README.md
-rw-r--r--   1 alex  staff   1258 16 Jul 07:10 api.ts
-rw-r--r--   1 alex  staff    395 26 May 10:52 car-browser.js
-rw-r--r--   1 alex  staff    371 26 May 10:52 car.js
drwxr-xr-x   7 alex  staff    224 16 Jul 08:59 cjs
drwxr-xr-x   8 alex  staff    256 16 Jul 08:59 esm
drwxr-xr-x   9 alex  staff    288 16 Jul 08:59 examples
-rw-r--r--   1 alex  staff     49 16 Jul 08:59 index.js
-rw-r--r--   1 alex  staff     64 16 Jul 08:59 indexed-reader
-rw-r--r--   1 alex  staff     49 16 Jul 08:59 indexer
-rw-r--r--   1 alex  staff     50 16 Jul 08:59 iterator
drwxr-xr-x  14 alex  staff    448 16 Jul 07:10 lib
-rw-r--r--   1 alex  staff   4515 16 Jul 08:59 package.json
-rw-r--r--   1 alex  staff     56 16 Jul 08:59 reader
drwxr-xr-x  16 alex  staff    512 16 Jul 08:24 test
-rw-r--r--   1 alex  staff   1252 16 Jul 07:36 tsconfig.json
drwxr-xr-x  10 alex  staff    320 16 Jul 08:59 types
-rw-r--r--   1 alex  staff     56 16 Jul 08:59 writer

Copy link
Member

@rvagg rvagg Jul 28, 2021

Choose a reason for hiding this comment

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

This only happens with bsd cp .. I mainly dev on Linux and also have coreutils installed on my mac machine so I never see (or have to think) about this behaviour! But consistency is good to avoid footguns, so all good.

Copy link
Member Author

@achingbrain achingbrain Jul 28, 2021

Choose a reason for hiding this comment

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

Aha, I had wondered if it might have been something like that since the published directory structure on npm is correct.

"build:types": "npm run build:copy && cd dist && tsc --build",
"build:copy": "mkdir -p dist/examples/ && cp -a tsconfig.json *.js *.ts lib test dist/ && cp examples/*.* dist/examples/",
"build:types": "tsc --build && mv types dist",
Copy link
Member Author

@achingbrain achingbrain Jul 16, 2021

Choose a reason for hiding this comment

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

No need to run the copy step twice if we're not building types from the dist folder.

"prepublishOnly": "npm run build",
"publish": "ipjs publish",
"lint": "standard",
Expand Down
6 changes: 2 additions & 4 deletions tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,8 @@
},
"exclude": [
"node_modules",
"esm",
"cjs",
"examples",
"index.js"
"dist",
"examples"
],
"compileOnSave": false
}