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

Remove dependencies on server and browser code in core/types #32567

Closed
wants to merge 2 commits into from

Conversation

joshdover
Copy link
Contributor

@joshdover joshdover commented Mar 6, 2019

Summary

The shared core/types directory cannot depend on core/public or core/server code or our TypeScript builds for production builds will conflict with one another (an issue that may be resolved by upgrading to Babel 7 w/ TS Support)

Additionally, the config service in new platform depends on legacy/util/package_json which is causing problems with conflicting TypeScript outputs in some PRs for core. This module does almost nothing, so this PR removes the dependency on it completely which also fixed the TS build issues.

@joshdover joshdover added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Feature:New Platform v8.0.0 v7.2.0 labels Mar 6, 2019
@joshdover joshdover requested a review from mshustov March 6, 2019 16:39
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform

@joshdover joshdover changed the title Remove dependency on legacy package_json util from core Remove dependencies on server and browser code in core/types Mar 6, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@joshdover
Copy link
Contributor Author

The core issue here is that for production builds, we build TS code for the server (tsconfig.json) as commonjs modules and for the browser (tsconfig.browser.json) as esmodules. If a module is imported by both the server and client-side builds, then the client-side build wins and the file is outputted as an esmodule, which Node cannot read.

In order to support TS code sharing (or even just type sharing) between server and browser code, we need to be outputting to a format that both Node and Webpack can read. In my testing it seems that outputting to CommonJS modules works for both environments. If there are no other issues with this approach, then this PR is unnecessary and we can simply change tsconfig.browser.json to remove the module format override.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@joshdover
Copy link
Contributor Author

Closing in favor of #32578

@joshdover joshdover closed this Mar 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:New Platform Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.2.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants