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

fix typescript version error #379

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

guilhermegregio
Copy link
Contributor

Solve error:

error An unexpected error occurred: "could not find a copy of typescript to link in /app/node_modules/webpack-config-single-spa-ts/node_modules".

Issue: #377

@changeset-bot
Copy link

changeset-bot bot commented May 11, 2023

🦋 Changeset detected

Latest commit: e3502c9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
webpack-config-single-spa-ts Patch
webpack-config-single-spa-react-ts Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@guilhermegregio
Copy link
Contributor Author

solved: error of builds @TheMcMurder, i saw i do not run tests locale and not create changes on lock file causing the build errors, verify again please

Guilherme Mangabeira Gregio added 2 commits May 11, 2023 09:48
@adripanico
Copy link

Change looks good. Could someone take a look to it, please? @joeldenning, maybe? Thanks

@joeldenning
Copy link
Member

joeldenning commented Sep 23, 2024

I tested this out locally and it didn't seem to fix the issue. However, I'm not sure because create-single-spa uses pnpm and the issue might be yarn-specific. I tried yarn linking the create-single-spa package, which might not be the same as if it were published to npm and installed that way.

Without confidence that the issue is fixed, I can't merge. webpack-config-single-spa-ts has a dependency on fork-ts-checker-webpack-plugin, which has a peerDependency on typescript, so I'm not sure whether moving typescript to devDependencies is the right thing to do.

@adripanico
Copy link

@joeldenning As far as I can see, webpack-config-single-spa-ts do have Typescript as peerDependency as well. So, at the end of the day, this change is just propagating the peerDepedency from fork-ts-checker-webpack-plugin to webpack-config-single-spa-ts and this one to the final app using it, right?

@joeldenning
Copy link
Member

The functionality of peerDependencies has changed so much and varies from package manager to package manager - I'm not sure what the correct approach is. Some have advocated for removing peerDependencies entirely because of how problematic they are.

Before merging, verifying that the change fixes the bug is required. I tried modifying webpack-config-single-spa-ts' package.json directly in node_modules and also yarn linking, and neither approach eliminated the error. Both of those approaches might not be equivalent to publishing to npm (linking notoriously is weird, and it's possible for package managers to reinstate the package.json during install), but I'm not sure how else to test it. Recommendations for how to verify the fix appreciated 🙏

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