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

[BUGFIX ts] ensure all packages have tsconfig & prepublish #6050

Merged
merged 1 commit into from
Apr 21, 2019

Conversation

runspired
Copy link
Contributor

@runspired runspired commented Apr 20, 2019

This allows npm publish --dry-run to complete in each package which should unblock us from releasing a beta.

A few follow up notes:

  • we aren't using rollup on most of the code anymore, we should restore this as a pre-publish step which would play nicer with consumption and embroider anyway
  • we should make our rollup code something available in build-infra so that all our packages can use it
  • we need to do a bit more work to ensure we aren't shipping types, I forget the details of the strategy that @mike-north had concocted but each package will need to be configured for that and this is something we must fix prior to this becoming a stable release.

@igorT
Copy link
Member

igorT commented Apr 20, 2019

Can you say more about parts of the code using rollup and parts not? I'm a bit confused by what you mean

@runspired
Copy link
Contributor Author

@igorT

Can you say more about parts of the code using rollup and parts not? I'm a bit confused by what you mean

Currently we only use rollup on the -private directory of ember-data package, but most of the code is in the @ember-data/store package. This means we have lost most of the benefits of rolling up and are now producing a lot more modules than before.

We should make all of the packages rollup their private directories, and ideally we should move this to being a prepublish step such that consuming applications get a single -private module when they install one of our packages and don't pay the cost of rollup at all.

This will also make things play nicer with embroider because embroider will not need to run our build step before being able to construct the graph correctly, as our modules will be in the correct form by the time embroider sees them.

@igorT
Copy link
Member

igorT commented Apr 20, 2019

Does that increase our build size? Lets open an issue with this context

@runspired
Copy link
Contributor Author

@igorT we should absolutely open an issue with context because yes, it will increase our build size and parse-time if not fixed.

@runspired runspired merged commit c34585f into master Apr 21, 2019
@runspired runspired deleted the fix-publishing branch April 21, 2019 00:17
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