Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Fix types for Oneof #216

Closed
wants to merge 2 commits into from
Closed

Conversation

skyline0705
Copy link

@skyline0705 skyline0705 commented Dec 14, 2019

In code, the typeof Oneof is new Rule, someone (like me) may use code like

const rule = webpackConfig.module.rule(lang).oneOf(type);
rule.uses.delete('something')

And I check the typing file, Type Oneof is not extends from Rule:https://github.com/neutrinojs/webpack-chain/blob/master/src/Rule.js#L40

@skyline0705 skyline0705 reopened this Dec 14, 2019
Copy link
Member

@edmorley edmorley left a comment

Choose a reason for hiding this comment

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

Hi! Thank you for the PR.

The tests are currently failing with:

$ tsc -p ./types/test/tsconfig.json
types/test/webpack-chain-tests.ts:301:8 - error TS2339: Property 'oneOfs' does not exist on type 'Module'.
301       .oneOfs
           ~~~~~~

Also, if there is a bug (that this PR is hoping to fix), it would be good to add a test for the case that was not previously working, to:
https://github.com/neutrinojs/webpack-chain/blob/master/types/test/webpack-chain-tests.ts

@skyline0705
Copy link
Author

@edmorley Thanks~~
I have add some test about this case, I don't know is this the best way to fix this bug. I have check the Code at above, the type 'OneOf' first must to be a 'Rule', but the typing file use 'ChainMap', which lose the properties at 'Rule'

@edmorley
Copy link
Member

@fimars @imyelo @Jetsly @loveky @bolasblack @opl-

Hi! Sorry for the mass @ mention - but you've all kindly contributed to the webpack-chain type definitions in the past, so I wondered if you'd be able to take a look at this PR? I'm not very familiar with TypeScript best practices, so not in the position to know if this is the correct fix for the issue mentioned above.

Thanks :-)

@edmorley edmorley added the bug label Dec 18, 2019
@loveky
Copy link
Contributor

loveky commented Dec 18, 2019

Created PR #218 to fix this type issue.

@skyline0705 also added your new test case in #218. Please help check if that solves your problem.

@skyline0705
Copy link
Author

skyline0705 commented Dec 18, 2019

Look good!I have tried to make oneOf as a Rule<Rule>, but it has some errors because I did not make Rule implements from Orderable

Thanks very much! @loveky

@edmorley edmorley closed this in 7fb3c67 Dec 19, 2019
@edmorley
Copy link
Member

Thank you everyone!

@edmorley edmorley added types and removed bug labels Jan 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

3 participants