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

Validate eui.d.ts after generating during build #1366

Merged
merged 2 commits into from
Dec 12, 2018

Conversation

chandlerprall
Copy link
Contributor

Summary

Adds a step to yarn build that validates the generated eui.d.ts file. This is prevent future regressions similar to #1359

Checklist

- [ ] This was checked in mobile
- [ ] This was checked in IE11
- [ ] This was checked in dark mode
- [ ] Any props added have proper autodocs
- [ ] Documentation examples were added
- [ ] A changelog entry exists and is marked appropriately
- [ ] This was checked for breaking changes and labeled appropriately
- [ ] Jest tests were updated or added to match the most common scenarios
- [ ] This was checked against keyboard-only and screenreader scenarios
- [ ] This required updates to Framer X components

@@ -20,7 +20,9 @@ function compileLib() {

// Use `tsc` to emit typescript declaration files for .ts files
console.log('Generating typescript definitions file');
execSync(`node ${path.resolve(__dirname, 'dtsgenerator.js')}`);
execSync(`node ${path.resolve(__dirname, 'dtsgenerator.js')}`, { stdio: 'inherit' });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

added stdio: 'inherit' here to give more visibility if any errors occur

// the import target is a `.d.ts` file which means it is hand-crafted and already added to the right places, don't modify
return path.join('@elastic/eui', params.importedModuleId);
} else if (isModuleIndex) {
if (isModuleIndex) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the fs.existsSync(importTargetDTs) was actually never hit, removed it

// if importing an `index` file
const isModuleIndex = path.basename(params.importedModuleId) === 'index';
let isModuleIndex = false;
Copy link
Contributor Author

@chandlerprall chandlerprall Dec 11, 2018

Choose a reason for hiding this comment

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

our structure of
icon/
index.ts
icon.tsx

means that index.ts should import from icon and export into @elastic/eui, but anything pointing to icon or icon/index should import from @elastic/eui (as the index file's exports target that namespace). Previously, this only worked correctly for import { IconType } from '../icon/index' and this change adds support for import { IconType } from '../icon'

@chandlerprall chandlerprall merged commit 3f50c45 into elastic:master Dec 12, 2018
@chandlerprall chandlerprall deleted the feature/validate-euidts branch December 12, 2018 16:45
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