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

Multiple Entry Points #71

Merged
merged 16 commits into from
Oct 15, 2016
Merged

Multiple Entry Points #71

merged 16 commits into from
Oct 15, 2016

Conversation

vramana
Copy link
Contributor

@vramana vramana commented Oct 9, 2016

  • Backends
  • Merlin
  • Refactor + comments

Fixes #36
Fixes #17

@jordwalke
Copy link
Collaborator

I'd love to hear more about what this change does (I'm not a Jenga expert)

@vramana
Copy link
Contributor Author

vramana commented Oct 9, 2016

This is the one of the two steps that will enable multiple entry points.

Currently, we don't track third party dependencies via the source files. We just read them from package.json and assume that they are being used. So we compile all dependencies regardless of whether they are necessary or not and include their artifacts while compiling and liniking. These assumptions worked okay till now because we are only dealing with one compiler. But when we have multiple entry points, some dependency may not be compiled by bsc or ocamlc. So, we compute ocamldep for each file and gather all the npm dependencies & ocamlfind dependencies and selectively compile them/include them while compiling/linking the source files. So, unused dependency will not be compiled will not compiled after this change. That is the gist of the PR.

I have made one subtle assumption temporarily. I assume that packages have not declared any extra dependencies that they have not used. Is this an okay assumption? I guess you can't.

@jordwalke
Copy link
Collaborator

I have made one subtle assumption temporarily. I assume that packages have not declared any extra dependencies that they have not used.

But you just said:

So, unused dependency will not be compiled will not compiled after this change.

And does this change track which files in my packages dependencies I use, or just the mere fact that I use the dependency in general?

@vramana
Copy link
Contributor Author

vramana commented Oct 10, 2016

Sorry for the confusing statement. We don't any compile unused packages from your source and dependencies as well. In the next step, we want to link whatever have been compiled so far but this is different function we need to recompute the dependencies all over again for your source and dependencies.

We can skip some of the computation here by assuming that packages don't over specify dependencies. And that's what I did in this PR. I know this feels very inconsistent. I'll change the code to just recompute everything.

@vramana vramana force-pushed the finer-dep branch 3 times, most recently from a115ef0 to 0f93afd Compare October 12, 2016 21:20
@vramana vramana changed the title Finer dependency tracking Multiple Entry Points Oct 12, 2016
@chenglou chenglou merged commit 3d1ae37 into reasonml-old:master Oct 15, 2016
jmorrell added a commit to jmorrell/RebelExampleProject that referenced this pull request Oct 15, 2016
reasonml-old/rebel#71 breaks the current example
repo requiring two small tweaks:

- `src/Test.re` needs to be renamed to the default target `src/Index.re`
- with multiple targets, we need to execute `app.native` instead of
  `app.out`
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.

3 participants