-
Notifications
You must be signed in to change notification settings - Fork 94
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
Add TypeScript typings for core & webpack #247
base: release-2.0
Are you sure you want to change the base?
Conversation
Thanks for starting work on this, @zcei! Will have a deeper look into it :) |
packages/webpack/index.d.ts
Outdated
export { createConfig, group, env, match } | ||
|
||
export function addPlugins(plugins: Plugin[]): WebpackBlock | ||
export function customConfig(configSnippet: Configuration): WebpackBlockUpdater |
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.
Actually returns a WebpackBlock
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.
I think we have to agree upon some terminology. The term block
, for instance, has always been a bit fuzzy. Providing explicit types is probably the ideal time to define it properly.
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.
Initially I was thinking that a block
is a self-contained, configurable portion of a Webpack Configuration - so actually entryPoint
is the block, not entryPoint()
, which could be a BlockInstance
or similar.
Then a block
is everything that has the signature (...args: any[]): BlockInstance
, where BlockInstance = (current) WebpackBlock
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.
Fair enough. I would follow your argumentation here, @zcei. BlockInstance
is not very catchy, but should do for now.
We need to discuss terminology here, since most terms haven't ever been clearly defined.
If someone has a better naming suggestion for |
Another term that might be discussable is Maybe it's rather a |
Update: Let's better discuss this in the issue (#246) than in the PR. |
203622e
to
7e8d49a
Compare
1f23d65
to
9d1535b
Compare
Hej guys 👋
I started tackling #246, for now only
@webpack-blocks/core
&@webpack-blocks/webpack
to have a foundation, so we can discuss naming etc.Afterwards we just need to add the proper
index.d.ts
to the blocks.If I'm not mistaken there shouldn't be other changes necessary, at least VSCode gave me type hints in the sample app already, but I'll list some here for research:
typings
topackage.json
(should be obsolete, as we comply toindex.d.ts
convention)index.d.ts
towebpack-blocks
which exports types from sub-modules (should be infered from sub-module typings)@types/
needs to be installed manually