-
Notifications
You must be signed in to change notification settings - Fork 59
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
[WIP] feat: support aegir.ts config file #994
Conversation
Added support for no-extension files, e.g. |
.aegir.ts
Outdated
@@ -0,0 +1,7 @@ | |||
const options: import('./src/types').PartialOptions = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's interesting, I didn't know you could do inline imports of types like this.
That said, is it better than import type { Foo } from './bar.js'
?
Also, I see there's no extension on ./src/types
- does this mean this file gets compiled to CJS in the background? What happens if you try to import an ESM-only module in your .aegir
file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let me look into that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That said, is it better than import type { Foo } from './bar.js'?
Nope! definitely not. fixing this.
Also, I see there's no extension on ./src/types - does this mean this file gets compiled to CJS in the background?
Not sure, but fixing this.
What happens if you try to import an ESM-only module in your .aegir file?
updated to
import type { PartialOptions } from './src/types.js'
import { isWritableStream } from 'is-stream'
console.log('isWritableStream: ', isWritableStream)
const options: PartialOptions = {
docs: {
entryPoint: 'utils'
}
}
export default options
and ran npm run lint
:
npm run lint
> [email protected] lint
> node src/index.js lint
Unexpected error while loading your config file
Unexpected error loading aegir config
/Users/sgtpooki/code/work/protocol.ai/ipfs/aegir/node_modules/ts-import/cache/Users/sgtpooki/code/work/protocol.ai/ipfs/aegir/.aegir.js:3
var is_stream_1 = require("is-stream");
^
Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/sgtpooki/code/work/protocol.ai/ipfs/aegir/node_modules/is-stream/index.js from /Users/sgtpooki/code/work/protocol.ai/ipfs/aegir/node_modules/ts-import/cache/Users/sgtpooki/code/work/protocol.ai/ipfs/aegir/.aegir.js not supported.
Instead change the require of index.js in /Users/sgtpooki/code/work/protocol.ai/ipfs/aegir/node_modules/ts-import/cache/Users/sgtpooki/code/work/protocol.ai/ipfs/aegir/.aegir.js to a dynamic import() which is available in all CommonJS modules.
at Object.<anonymous> (/Users/sgtpooki/code/work/protocol.ai/ipfs/aegir/node_modules/ts-import/cache/Users/sgtpooki/code/work/protocol.ai/ipfs/aegir/.aegir.js:3:19)
at /Users/sgtpooki/code/work/protocol.ai/ipfs/aegir/node_modules/ts-import/dist/main.js:34:55
at async Object.load (/Users/sgtpooki/code/work/protocol.ai/ipfs/aegir/node_modules/ts-import/dist/main.js:34:20)
at async loadTs (file:///Users/sgtpooki/code/work/protocol.ai/ipfs/aegir/src/config/user.js:61:11)
at async Object.search (/Users/sgtpooki/code/work/protocol.ai/ipfs/aegir/node_modules/lilconfig/dist/index.js:126:37)
at async config (file:///Users/sgtpooki/code/work/protocol.ai/ipfs/aegir/src/config/user.js:110:26)
at async main (file:///Users/sgtpooki/code/work/protocol.ai/ipfs/aegir/src/index.js:48:22) {
code: 'ERR_REQUIRE_ESM'
}
Node.js v17.4.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems like ts-import doesn't support esm, which would break things
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adding a test for this..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this is what I thought might happen. It might be worth opening an issue on the ts-import repo about it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this was fixed on Dec 5: radarsu/ts-import#24 (comment) with version 4.0.0-beta.10
. I won't be able to get back to this PR for a bit, but documenting this
265a8bf
to
2cbe5c6
Compare
pushed the new tests, but it's currently failing with the error specified above. |
closing this for now because I don't have the bandwidth to take this on |
also tested against ipfs-shipyard/pinning-service-compliance by running