Skip to content
This repository has been archived by the owner on Aug 30, 2021. It is now read-only.

fix(gulp): Fixing/updating templatecache gulp task #1160

Merged

Conversation

tfernandez
Copy link
Contributor

The transformUrl replace function has been removed from templatecache gulp task.

The client view routes include the string "/client" in the url from mean.js 0.4.2 and the browser view request is not matching with the angular template cache url, so all the templates cache is being ignored at startup.

The task has been updated.

@ilanbiala
Copy link
Member

@tfernandez have you tested and confirmed it's working now?

@tfernandez
Copy link
Contributor Author

@ilanbiala, yes, is tested and working now, from meanjs 0.4.2 the client view route matches with the real path, the transformUrl function is generating a wrong url now.

To reproduce the error just execute "gulp prod" and check the "build/templates.js" file, the urls generated does not match with the html loaded by the browser causing the browser loads all the templates on demand and ignoring the cache.

Probably @rhutchison can confirm the problem.

@ilanbiala
Copy link
Member

@tfernandez any reason the regex might have existed in the first place? Maybe for cross-OS support? What platform(s) have you tested on?

@tfernandez
Copy link
Contributor Author

@ilanbiala the regex was used inside the "transformUrl" function to replace the "/client/" string from the template realpath with "/" and generate a new valid html view url.

Original/real path:
/modules/core/client/views/home.client.view.html

Generated/transformed url:
/modules/core/views/home.client.view.html

However this replacement must not be done from meanjs 0.4.2 and the regex is unnecessary.

Is tested on OS X and CentOS

@rhutchison
Copy link
Contributor

@tfernandez this needs to be tested on Windows.

@trendzetter trendzetter mentioned this pull request Jan 30, 2016
6 tasks
@ilanbiala
Copy link
Member

@tfernandez have you tested on windows?

@tfernandez
Copy link
Contributor Author

@ilanbiala tested this afternoon on Windows 10 in cmd and cygwin

The issue exists in the same way and is solved with the same solution.

@ilanbiala
Copy link
Member

@tfernandez solved with what solution?

@tfernandez
Copy link
Contributor Author

@ilanbiala the problem is the template cache gulp task is removing "/client" from the templatecache uris before saving it, and the solution is remove the regex and the transformUrl method from the task (the target of this pull request)

@ilanbiala
Copy link
Member

@tfernandez what OSes has this been tested on?

@tfernandez
Copy link
Contributor Author

@ilanbiala has been tested on CentOs 6/7, OSX El Capitan and Windows 10

@ilanbiala ilanbiala added this to the 0.5.0 milestone Feb 5, 2016
@ilanbiala ilanbiala self-assigned this Feb 5, 2016
@ilanbiala
Copy link
Member

@tfernandez SGTM, just fix the commit message to follow our guidelines in contributing.md, and I'll merge.

@codydaig
Copy link
Member

codydaig commented Feb 6, 2016

Update commit message, then LGTM.

@tfernandez tfernandez force-pushed the feature/fixing-gulp-templatecache branch from b7b0dfc to 59d9347 Compare February 9, 2016 15:46
@tfernandez tfernandez changed the title Fixing/updating templatecache gulp task fix(gulp): Fixing/updating templatecache gulp task Feb 9, 2016
@tfernandez
Copy link
Contributor Author

Changed the commit message

@codydaig
Copy link
Member

codydaig commented Feb 9, 2016

LGTM

@rhutchison
Copy link
Contributor

LGTM - merge.

It wasn't very clear on why the change was needed. I did some testing/digging.

Regression from #758

ilanbiala added a commit that referenced this pull request Feb 10, 2016
…cache

fix(gulp): Fix templatecache gulp task
@ilanbiala ilanbiala merged commit 8a705a9 into meanjs:master Feb 10, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants