Skip to content
This repository has been archived by the owner on Mar 1, 2019. It is now read-only.

Template compiler fix #251

Conversation

ndreckshage
Copy link

@ndreckshage ndreckshage commented Jul 25, 2016

Overview

The compiler seems to break for me because module.dest undefined, but templatePath not referenced anyways?

Additionally, only the dist folder is in npm install. Build scripts adds files arr to package.json before publish, acting as a more restrictive npmignore

Needed to get redux adaptor working https://github.com/HumbleSpark/tungsten-redux-adaptor

@codecov-io
Copy link

codecov-io commented Jul 25, 2016

Current coverage is 85.61% (diff: 100%)

No coverage report found for master at 370fe00.

Powered by Codecov. Last update 370fe00...0658d2a

rm scoped package name from pr
@andrewrota
Copy link
Contributor

@ndreckshage we've tried to expose everything we need to precompile templates on the main exports object so we don't need to deep-link into the repository. That's also why the build only publishes dist files...custom builds should download the entire repo (and its devdependencies which normally don't get included). We've found this reduces the surface area for potential breaking API changes.

I'm using require('tungstenjs').precompiler for a webpack loader in my boilerplate app; would a similar approach work here? If not could we expose the specific functions you need instead?

Looking forward to the redux adaptor!

@ndreckshage
Copy link
Author

@andrewrota dist should work then, thanks!

Although I think there is still the issue with var templatePath in precompiler. Current precompiler in npm doesn't seem to work.

Additionally, I think there are a few functions in shared that are not exported in simplified API (ie, https://github.com/wayfair/tungstenjs/blob/master/adaptors/shared/render_queue.js) that would need to be for an adapter (redux-adapter, or for backbone-adapter to live outside of this repo, etc).

Will update pr to just add additional export(s) I need for now.

@andrewrota
Copy link
Contributor

Closing this. I don't mind discussing it further, but I don't think this PR addresses the API problems.

@andrewrota andrewrota closed this Oct 18, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants