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

Semver dependency on typescript in minor range is a breaking change: TypeError: getFileSystemEntries is not a function #49

Closed
theseyi opened this issue Aug 18, 2017 · 2 comments

Comments

@theseyi
Copy link
Contributor

theseyi commented Aug 18, 2017

broccoli-typescript-compiler has a permissive package.json that depends on TypeScript version ^2.3.3. This minor range dependency currently allows Yarn to install TS 2.5.0 which is at release candidate rc stage.

https://registry.yarnpkg.com/typescript revision: 1052-fb9eebd5b091190020c3c186ce299827

{
latest: "2.4.2",
next: "2.6.0-dev.20170817",
beta: "2.0.0",
rc: "2.5.0",
insiders: "2.4.2-insiders.20170719"
},

Unfortunately, this actually breaks the broccoli build step because there is an incompatible api change:
in typescript.js, the function signature matchFiles in @2.5.0 rc, accepts an additional argument depth:

function matchFiles(path, extensions, excludes, includes, useCaseSensitiveFileNames, currentDirectory, depth, getFileSystemEntries) {
...
}

https://github.com/Microsoft/TypeScript/blob/v2.5-rc/src/compiler/core.ts#L2073
versus the current/latest @2.4.2 signature

function matchFiles(path, extensions, excludes, includes, useCaseSensitiveFileNames, currentDirectory, getFileSystemEntries) {
...
}

https://github.com/Microsoft/TypeScript/blob/v2.4.2/src/compiler/core.ts#L2023
You'll notice that the depth parameter was not previuosly present

This cause the broccoli build to throw:

TypeError: getFileSystemEntries is not a function

EDIT: updated with TS repo source links

@rwjblue
Copy link
Collaborator

rwjblue commented Aug 18, 2017

I reported the breaking change to the typescript folks in microsoft/TypeScript#17891. However, we will still have to address to mitigate failures for all of our users.

This repeated pattern of breaking changes within the same major version in typescript is quite annoying... 😡

@theseyi
Copy link
Contributor Author

theseyi commented Aug 18, 2017

Thanks for taking a look @rwjblue . Speaking of failures for users, would the PR patch #50 be a quick way to mitigate the failures. At least until a lasting resolution is made? I assume other users have a critical dependency on this since it will break all continuous integration builds, and any clean builds

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

No branches or pull requests

2 participants