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

Build cleanup and output declarations #343

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Build cleanup and output declarations #343

wants to merge 1 commit into from

Conversation

lynchbomb
Copy link
Member

@lynchbomb lynchbomb commented May 31, 2018

Hang tight on review of this PR. Im going to squash the declarations files and expose a index.d.ts.

Spiking this now. TBH there more than likely is an easier way to output generated ts declaration files. Any insight is more than welcome :)

Copy link
Collaborator

@krisselden krisselden left a comment

Choose a reason for hiding this comment

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

you need a types in the package.json entry that points at the output declarations and also, declarations don't really need to be flattened.

@lynchbomb lynchbomb force-pushed the lynchbomb branch 6 times, most recently from 04d0b57 to ba49a95 Compare June 4, 2018 20:02
@lynchbomb
Copy link
Member Author

lynchbomb commented Jun 4, 2018

@krisselden should be good to merge. I also wan't to confirm we still actually need Bublé (rollup-plugin-buble) since we already have ES2015 as an output target via TypeScript.

@lynchbomb lynchbomb force-pushed the lynchbomb branch 2 times, most recently from a2f5fe7 to 7214d35 Compare June 4, 2018 20:15
const compiled = typescript(src, {
throwOnError: process.env.EMBER_ENV === 'production',
});

const compiledDeclarations = typescript('lib', {
tsconfig: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

In general we shouldn't need to pass tsconfig here, it should just be able to use our top level tsconfig.json.

Copy link
Member Author

Choose a reason for hiding this comment

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

If we don't explicitly flag for declaration here and default to tsconfig with declaration: true we will have to cherry-pick out the .d.ts files for tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually this brings up a good point as to; do we actually need to output tests within dist?

Copy link
Member Author

Choose a reason for hiding this comment

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

tsconfig.json Outdated
@@ -12,8 +12,8 @@
"backburner": ["lib/index.ts"]
}
},
"files": [
"file": [
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't grok this change, I didn't think file was an option for tsconfig.json?

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.

None yet

3 participants