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

mocha: Use correct function for merging lint configuration #1219

Merged
merged 1 commit into from Nov 23, 2018
Merged

mocha: Use correct function for merging lint configuration #1219

merged 1 commit into from Nov 23, 2018

Conversation

edmorley
Copy link
Member

The refactor in #1182 inadvertently used babel-merge's merge() to merge ESLint configuration in this preset rather than deepmerge, since the existing merge import was not the same as in other presets from which the lint rule merging block was copied.

In a later PR I think we should stop exporting a merge function from @neutrinojs/compile-loader named merge, to avoid the reduce the chance of this kind of naming conflict.

Fixes:
taskcluster/taskcluster-web-server#59 (comment)

The refactor in #1182 inadvertently used babel-merge's `merge()` to
merge ESLint configuration in this preset rather than deepmerge, since
the existing `merge` import was not the same as in other presets from
which the lint rule merging block was copied.

In a later PR I think we should stop exporting a merge function from
`@neutrinojs/compile-loader` named `merge`, to avoid the reduce the
chance of this kind of naming conflict.

Fixes:
taskcluster/taskcluster-web-server#59 (comment)
@edmorley edmorley added the bug label Nov 23, 2018
@edmorley edmorley added this to the v9 milestone Nov 23, 2018
@edmorley edmorley self-assigned this Nov 23, 2018
Copy link
Member

@helfi92 helfi92 left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for the quick fix.

@edmorley edmorley merged commit aac7682 into neutrinojs:master Nov 23, 2018
@edmorley edmorley deleted the fix-mocha-lint-merge branch November 23, 2018 14:48
@helfi92
Copy link
Member

helfi92 commented Nov 23, 2018

@edmorley Any idea when this will be published?

@edmorley
Copy link
Member Author

Publishing new releases of the monorepo is pretty broken on Windows at the moment, so I've been leaving it to Eli to publish new versions. Happy to have a new beta published whenever :-)

@helfi92
Copy link
Member

helfi92 commented Nov 23, 2018

A new beta sounds good to me :)

edmorley added a commit that referenced this pull request Nov 26, 2018
Since it's no longer a custom implementation and instead just the
`babel-merge` package exported. As such it makes more sense for
packages to import `babel-merge` directly iff they need it.

This also avoids the naming clash between `merge` from this preset
and the typical import style used for `deepmerge` in this repository,
that contributed to the bug fixed by #1219.
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.

None yet

2 participants