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

Add support for nested main field selectors #218

Merged
merged 2 commits into from Aug 6, 2022

Conversation

aaronadamsCA
Copy link
Contributor

Fixes #217.

There's some slightly repetitive code, I just wasn't sure where a common function should live, so I left the minor copy-paste.

@codecov
Copy link

codecov bot commented Jul 30, 2022

Codecov Report

Merging #218 (0a2d6f4) into master (f420039) will increase coverage by 0.31%.
The diff coverage is 66.66%.

❗ Current head 0a2d6f4 differs from pull request most recent head b96b740. Consider uploading reports for the commit b96b740 to get more accurate results

@@            Coverage Diff             @@
##           master     #218      +/-   ##
==========================================
+ Coverage   67.97%   68.28%   +0.31%     
==========================================
  Files           9        9              
  Lines         306      309       +3     
  Branches       93       95       +2     
==========================================
+ Hits          208      211       +3     
  Misses         92       92              
  Partials        6        6              
Impacted Files Coverage Δ
src/config-loader.ts 83.33% <ø> (ø)
src/filesystem.ts 73.91% <ø> (ø)
src/match-path-async.ts 69.35% <66.66%> (+1.02%) ⬆️
src/match-path-sync.ts 79.06% <66.66%> (+0.49%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us.

@jonaskello
Copy link
Member

jonaskello commented Jul 30, 2022

Altough I would prefer to keep webpack specific stuff in the webpack plugin, resolving nested mainfields in package.json cannot be done in the plugin so I guess we need to add this.

@jonaskello
Copy link
Member

The jsdoc currently says this about the mainFields param:

@param mainFields A list of package.json field names to try when resolving module files.

@aaronadamsCA Could you update that comment to reflect the new function? Also the same comment is in the readme.

@aaronadamsCA
Copy link
Contributor Author

aaronadamsCA commented Aug 1, 2022

Altough I would prefer to keep webpack specific stuff in the webpack plugin, resolving nested mainfields in package.json cannot be done in the plugin so I guess we need to add this.

If you prefer, I can update the downstream PR instead to just throw an error if the script reaches an array. This syntax does not appear to be in wide use anymore; it seems to relate to a proposal that never went anywhere.

The jsdoc currently says this about the mainFields param:

@param mainFields A list of package.json field names to try when resolving module files.

@aaronadamsCA Could you update that comment to reflect the new function? Also the same comment is in the readme.

Done.

@jonaskello
Copy link
Member

I think we could have this for completeness so let's merge it and then look into the downstream PR.

@jonaskello jonaskello merged commit 9124aa6 into dividab:master Aug 6, 2022
@jonaskello
Copy link
Member

Released in 4.1.0

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.

Correct mainFields type
2 participants