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
Node and requirejs typings messing up my build #2142
Comments
Can you explain exactly how this type dependency is conflicting in your app? What issues are you seeing here? |
After updating to the latetest rollup version, my build suddenly failed while compiling:
I was wondering why the Node typings were installed and I found out, they where included by the rollup package.json. Without the node or requirejs typings, I don't get the error. I'm using Angular 5.0.1 with TS 2.7.2 |
@EricSch it seems like this is a particular aspect of your app that the Node types are not compatible here. I'd suggest adding a custom exclude to your tsconfig.json file to ignore the types - We do need to include the Node types as a dependency for our types to work in the first place, so removing isn't really an option unfortunately. For as amazing as TypeScript is it has never quite got JS modularity right :P |
@guybedford Thanks for answering, will try that, And I would say, Typescript has to fight with the JS modularity mess. There are too many different module systems to get everything in every case right 😉 |
We could also make the |
If we move them entirely to the dev dependencies, this will leave people with broken types and no information what they have to do to fix that. |
Yeah peerDependencies doesn't seem to quite solve it either unfortunately. Marking as a wontfix but happy to discuss further. |
@guybedford Excluding the node types doesn't work. TS includes automatically every typings in @types. |
@EricSch the fact that the "node" type specifically is breaking for you in a Node workflow is a sign that the RequireJS typing approach is just plain wrong. In particular I would argue that this line - https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/requirejs/index.d.ts#L422 is invalid as it is altering an environment invariant, while RequireJS in NodeJS will not actually override the |
@guybedford I removed the requireJs typings from our package.json. They were obsolete. seems an issue with the typings. So, those node typings are giving me only troubles. Specially annoying because I don't need them. |
@EricSch you can fix that by locking the broken Node typing to the last working version. The hard constraint we have here is to ensure Rollup can export working typings that don't give users warnings. And leaving out the Node typing will cause warnings in VSCode and when validating types. The only alternative I can possibly think of would be to inline our Node typings, specifically inlining the entire definition of EventEmitter in src/rollup/types.d.ts. |
Hi, |
Having node types in rollup leads to another nasty issue: it makes code compile that shouldn't. For example, with this
And indeed, if I forget from what spec const a = ['foo', 'bar']
console.log(a.find(s => s.indexOf('oo') !== -1)) tsc helpfully complains:
Now I know I have to either get rid of it or import a polyfill. However, if I want to bundle this project with rollup, due to @types/node being installed I get:
I.e. code compiles fine now and breaks at runtime. Ideas on how I can address this? |
In case anyone else stumbles on this: found workaround in microsoft/TypeScript#18588 (comment) via microsoft/TypeScript#17042. In package.json:
|
Hey folks. This is a saved-form message, but rest assured we mean every word. The Rollup team is attempting to clean up the Issues backlog in the hopes that the active and still-needed, still-relevant issues bubble up to the surface. With that, we're closing issues that have been open for an eon or two, and have gone stale like pirate hard-tack without activity. We really appreciate the folks have taken the time to open and comment on this issue. Please don't confuse this closure with us not caring or dismissing your issue, feature request, discussion, or report. The issue will still be here, just in a closed state. If the issue pertains to a bug, please re-test for the bug on the latest version of Rollup and if present, please tag @shellscape and request a re-open, and we'll be happy to oblige. |
Sorry for the inconvenience, Rollup@2 will finally fix this: #3395 |
Solved in rollup@2.0.0 |
I solved this problem by deleting "requirejs" from "tsconfig.json" > "compilerOptions" > "types" for Browserify (CommonJS) version And I add "node" to "types":
But for AMD version I added "requirejs" to "types" package.json
tsconfigs/tsconfig.debug.json
tsconfigs/tsconfig.json
tsconfigs/tsconfig.release.json
public/index.html
src/client/main.ts
src/client/RequireConfig.ts
|
Since the Node typings are moved to "dependencies" in package.json, they conflict with the requirejs typings
"@types/requirejs": "2.1.31"
.I understand, it isn't exactly a rollup issue, but have they to be in "dependencies" and not like before in the "devDependencies" section?
Thanks for your great work
The text was updated successfully, but these errors were encountered: