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

Support for type definition files #188

Merged
merged 41 commits into from
Oct 30, 2020
Merged

Support for type definition files #188

merged 41 commits into from
Oct 30, 2020

Conversation

TwitchBronBron
Copy link
Member

@TwitchBronBron TwitchBronBron commented Sep 23, 2020

This adds support for loading type definition files (d.bs extension).

Notable changes:

  • load type definition files during file parse.
  • cache type files to reduce excess file system loading.
  • generate type definitions during transpile when emitDefinitions is true in the bsconfig

@TwitchBronBron
Copy link
Member Author

Forgot to mark this as "draft". It's not ready yet.

src/Program.ts Outdated Show resolved Hide resolved
src/Program.ts Outdated Show resolved Hide resolved
src/Program.ts Outdated Show resolved Hide resolved
@@ -95,6 +99,20 @@ export class Body extends Statement {
return result;
}

getTypedef(state: TranspileState) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not having a "transpilation mode", even as a static flag somewhere, to control that instead of doubling half the transpilation logic with those getTypedef methods?

Copy link
Member Author

Choose a reason for hiding this comment

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

I considered that, but most expressions and statements will not emit any typedef information. Having a separate function allows me to detect which items we should even walk in the first place (and avoid needless transpile recursing). See
if ('getTypedef' in statement) { a few lines below this comment.

Also, if most of the transpile function can be reused, I'm already calling it within the getTypedef method so we don't actually duplicate any logic. (see comment statement). Meanwhile, something like a NamespaceStatement doesn't actually transpile itself, so that's where having a separate function keeps the logic clean between the two concepts (transpile and getTypedef). Or ClassStatement, whose transpile function is extremely complex and doesn't have any similarities with what its typedef would look like.

src/Scope.ts Outdated Show resolved Hide resolved
this.needsTranspiled = true;
}
this.isTypedef = this.extension === '.d.bs';
if (!this.isTypedef) {
this.typedefKey = util.getTypedefPath(this.pathAbsolute);
Copy link
Contributor

Choose a reason for hiding this comment

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

Feels a bit wasteful to calculate for every file - should it be undefined until it "has" a typedef?

Copy link
Member Author

Choose a reason for hiding this comment

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

We need the key for every file right away because we use that key to subscribe to typedef file changes (add/update/delete of typedefs).

src/files/BrsFile.ts Outdated Show resolved Hide resolved
src/files/BrsFile.ts Outdated Show resolved Hide resolved
src/files/BrsFile.ts Outdated Show resolved Hide resolved
src/files/BrsFile.ts Outdated Show resolved Hide resolved
src/files/BrsFile.ts Outdated Show resolved Hide resolved

let allDependencies = this.getAllDependencies()
//skip typedef files
.filter(x => extname(x) !== '.d.bs');
Copy link
Contributor

Choose a reason for hiding this comment

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

Should they be replaced by .brs files?

Copy link
Member Author

@TwitchBronBron TwitchBronBron Oct 30, 2020

Choose a reason for hiding this comment

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

No, the .bs/.brs files are already there. For every .bs/.brs file, the XmlFile also adds the corresponding .d.bs file as a dependency during initialization (see this new block of code)

src/util.ts Show resolved Hide resolved
@TwitchBronBron
Copy link
Member Author

I believe this is functional enough for an initial release. I'm sure we'll run into some quirks, but luckily enough, this is an opt-in feature so it shouldn't have too many adverse effects for teams who aren't opting in to this feature.

@TwitchBronBron TwitchBronBron merged commit 478eb30 into master Oct 30, 2020
@TwitchBronBron TwitchBronBron deleted the type-definitions branch October 30, 2020 19:03
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.

2 participants