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

Addon-shim was missing dependencies #1282

Merged
merged 3 commits into from
Nov 15, 2022
Merged

Conversation

NullVoxPopuli
Copy link
Collaborator

This came up when I moved broccoli-funnel to nohoist in a yarn project -- got an error about broccoli-funnel not being found

yarn.lock Outdated Show resolved Hide resolved
@ef4
Copy link
Contributor

ef4 commented Nov 15, 2022

Does broccoli-node-api break if you don't change it? Because it's types-only and I don't think our own types refer to them, so it seems like it could stay a devDependency.

As for the other one, we should also figure out why we don't have a lint rule to catch this.

@NullVoxPopuli
Copy link
Collaborator Author

yeah that one is fine as a devDep, I just got carried away it my hastily made PR.

I'll look for a lint for the imports and open a separate PR for that (this is something $work wants, too, so hopefully whatever I find works for both places!)

@ef4
Copy link
Contributor

ef4 commented Nov 15, 2022

I'll look for a lint for the imports and open a separate PR for that (this is something $work wants, too, so hopefully whatever I find works for both places!)

It's normally handled by node/no-extraneous-import and node/no-extraneous-require.

People seem to be moving from eslint-plugin-node to eslint-plugin-n, a fork that is more maintained.

@NullVoxPopuli
Copy link
Collaborator Author

yeah, eslint-plugin-node barely supports anything these days (except old cjs)

@ef4 ef4 merged commit 933794b into embroider-build:main Nov 15, 2022
@NullVoxPopuli NullVoxPopuli deleted the fix-deps branch November 15, 2022 17:02
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.

3 participants