Maintain a packaged theme alongside semantic #2101

gabro · 2015-04-11T17:46:05Z

Hi all, I'm building a custom packaged theme for my project and I'd like to store it in the same repo where the site source code is. Here's my directory layout:

├── Gulpfile.js
├── dist
├── node_modules
├── package.json
├── semantic
├── semantic.json
├── src
├── theme.config
└── themes
    ├── default
    └── mytheme

Since whenever I update semantic with npm the semantic/src/themes directory gets overridden, I thought of putting the my theme (and the default one) in a themes directory at root level.

Then I configured my semantic.json as follows:

{
  "base": "semantic/",
  "paths": {
    "source": {
      "config": "../theme.config",
      "definitions": "src/definitions/",
      "site": "src/site/",
      "themes": "../themes"
    },
    "output": {
      "packaged": "dist/",
      "uncompressed": "dist/components/",
      "compressed": "dist/components/",
      "themes": "dist/themes/"
    },
    "clean": "dist/"
  },
  "permission": false,
  "rtl": false,
  "version": "1.11.6"
}

so that I could put theme.config and themes under version control and happily ignore the semantic folder altogether.

Unfortunately, it looks like when I build semantic (with gulp build), it's still using the semantic/src/theme.config file.

Did I get something wrong?
And in general, is there any preferred strategy for putting only my custom packaged theme under version control being able to safely update the semantic npm package?

Thank you

The text was updated successfully, but these errors were encountered:

jlukic · 2015-04-11T18:14:54Z

Hi, the themes folder shouldnt be overwritten. I apologize about that, its most likely a bug.

I'll have to take a look at update script. I think this is probably an issue with the settings for wrench when copying files similar to #1845

gabro · 2015-04-11T22:52:11Z

Hi and than you for the quick feedback!

What I've tried is to clone a repo with a pre-existing semantic directory containing only src/themes/mytheme/ and src/theme.config.
After running npm install and the semantic installer, the mytheme directory is gone (git marks it as deleted) and theme.config is back to the default.

My goal is rather straightforward: maintain my theme in the site repo and install/update semantic on top of it (with npm install/update). I'm open to any approach, as long as npm install after cloning the repo will be the only necessary operation to do.

jlukic · 2015-04-13T04:31:06Z

I'll test this out..

…ath) #2101

jlukic · 2015-04-13T18:03:44Z

I've verified this is fixed from a fresh install. Will be releasing patched version today.

gabro · 2015-04-13T18:29:40Z

awesome, thank you for the quick fix!

gabro · 2015-04-13T19:35:50Z

Hi @jlukic, I just tried the version 1.11.17 (via npm install Semantic-Org/Semantic-UI) and I'm still facing the same issue. Steps to reproduce:

I clone my repo, which contains a semantic directory with the following layout

semantic
└── src
    ├── theme.config
    └── themes
        └── mytheme

I run npm install
I perform an "Automatic" installation of Semantic (extending my current settings)

As a result, the mytheme folder is gone.
Here's my (shortened) git status right after

modified:   semantic.json
deleted:    semantic/src/themes/mytheme/collections/message.overrides
deleted:    semantic/src/themes/mytheme/collections/message.variables
...

Any ideas?

jlukic · 2015-04-13T19:37:06Z

How recently did you install? I just pushed 1.11.7 a few minutes ago.

Does your semantic.json show 1.11.7?

gabro · 2015-04-13T19:41:24Z

Yes it shows 1.11.7.
I tested it against master 10 minutes ago (pointing npm to the github repo directly), since it wasn't released on npm yet.

jlukic · 2015-04-13T19:43:10Z

Debugging locally with

npm install [email protected]
mkdir semantic/src/themes/fake
npm install [email protected]
cd semantic/src/themes/  && ls

jlukic · 2015-04-13T19:47:15Z

Oh wait i havent published 1.11.7 to npm yet. Just did now 😄

I just confirmed with the above procedure on an empty directory that user themes should not be touched with update.

gabro · 2015-04-13T19:57:12Z

I'm trying right now (1.11.7 fresh from npm) and I still have the same issue.
The main difference from your test case and mine is the prior existence of a Semantic installation.

I confirmed this behavior by doing

mkdir -p testproject/semantic/src/themes/fake
cd testproject
npm install [email protected]
cd semantic/src/themes && ls

(fake is gone now)

The idea is that my theme is in place before semantic is ever installed via npm so that I can ship it with the rest of the project and run a fresh npm install on top of it.

jlukic · 2015-04-13T19:59:19Z

Ohh you are talking about from the install path. Not the upgrade path. This is a whole different bundle of code.

gabro · 2015-04-13T20:00:55Z

Yep, exactly. Does it make sense? Or am I getting this wrong?

gabro · 2015-04-13T20:05:26Z

I mean, I get that when installing the themes you need to dump the whole themes directory into the path (since it doesn't exist yet).
I think a preserving what's inside (i.e. a merging behavior) instead of replacing it altogether would be a sensible behavior.

Or, as always, I'm open to other suggestions for shipping my theme :)

jlukic · 2015-04-13T20:28:13Z

I just need to release another patch. I hadn't considered modifying the behavior of install, only update.

gabro · 2015-04-13T20:34:45Z

Cool, thank you!
Actually, I don't know if you noticed but testing against master gave me this error during the Semantic installation

Copying UI themes

fs.js:695
  return binding.stat(pathModule._makeLong(path));
                 ^
TypeError: path must be a string
    at Object.fs.statSync (fs.js:695:18)
    at Object.exports.copyDirSyncRecursive (/Users/gabro/Work/buildo/lexdoit/webtest/node_modules/semantic-ui/node_modules/wrench/lib/wrench.js:244:23)
    at /Users/gabro/Work/buildo/lexdoit/webtest/node_modules/semantic-ui/tasks/install.js:278:16
    at /Users/gabro/Work/buildo/lexdoit/webtest/node_modules/semantic-ui/node_modules/gulp-prompt/index.js:24:5
    at Object.<anonymous> (/Users/gabro/Work/buildo/lexdoit/webtest/node_modules/semantic-ui/node_modules/gulp-prompt/node_modules/inquirer/lib/inquirer.js:81:7)
    at /Users/gabro/Work/buildo/lexdoit/webtest/node_modules/semantic-ui/node_modules/gulp-prompt/node_modules/inquirer/node_modules/async/lib/async.js:232:13
    at /Users/gabro/Work/buildo/lexdoit/webtest/node_modules/semantic-ui/node_modules/gulp-prompt/node_modules/inquirer/node_modules/async/lib/async.js:142:25
    at /Users/gabro/Work/buildo/lexdoit/webtest/node_modules/semantic-ui/node_modules/gulp-prompt/nod
> [email protected] install /Users/gabro/Work/buildo/lexdoit/webtest/node_modules/node-sass
> node scripts/install.js

jlukic · 2015-04-13T20:36:56Z

Yeah, i'm actively working on a fix in master.

gabro · 2015-04-13T20:38:32Z

Great, I really appreciate the live support :D Thanks for the amazing work!

gabro · 2015-04-13T20:40:29Z

I just tested it against the last commit and it works as expected! ;)

jlukic · 2015-04-13T20:41:09Z

Can you test against master now, i think everything is ready. If everything works out I'll package up a new patch release.

gabro · 2015-04-13T20:45:18Z

See my previous comment, it works after testing against 2df4e82!

Thank you again very much for the prompt support!

Semantic-Pusher-Robot · 2015-04-13T20:46:10Z

Thanks bundling it up.
-evil pusher robot

# By jlukic # Via jlukic * 'master' of https://github.com/Semantic-Org/Semantic-UI: Build 1.11.8 Add Semantic-Org#2102 to 1.x Version tick 1.11.8 Semantic-Org#2101 finish fix for npm install Fix theme path Semantic-Org#2101 Install path changes Semantic-Org#2101 Publish tasks fail with latest git due to add syntax Rlsnotes 1.11.7 Build version 1.11.7 Backport flex fix for cards Semantic-Org#2069 Semantic-Org#2021 Semantic-Org#2105 More name consolidation Semantic-Org#2101 More obvious name for merge copy Semantic-Org#2101 NPM Update should only update default theme folder (not whole theme path) Semantic-Org#2101 Remove console.log from sticky Semantic-Org#2107

* next: (32 commits) Semantic-Org#2092 rebuild dist Add comments to search settings Rlsnotes Semantic-Org#2059 Semantic-Org#2070 Fixes Semantic-Org#2059 Semantic-Org#2070 rewrite object search used in search module to be much less obtuse Fix debug Semantic-Org#2059 Semantic-Org#2070 Add mock response async, yahoooo related to Semantic-Org#2059 Semantic-Org#2070 Semantic-Org#2059 Finish ability to mock responses in API Similar Semantic-Org#2092 add ability to mock server response using function Semantic-Org#2092 RLSNOTES & Cred Remove debug Semantic-Org#2092 Semantic-Org#2092 Add onResponse callback to allow for response transformations Semantic-Org#2070 - dont continue loop unnecessarily, clarify variable names Add Semantic-Org#2113 Add accordion on event, update rlsnotes Publish tasks fail with latest git due to add syntax Rlsnotes 1.11.7 Build version 1.11.7 Backport flex fix for cards Semantic-Org#2069 Semantic-Org#2021 Semantic-Org#2105 More name consolidation Semantic-Org#2101 More obvious name for merge copy Semantic-Org#2101 NPM Update should only update default theme folder (not whole theme path) Semantic-Org#2101 ...

jlukic added the Evaluating Bug / Change label Apr 11, 2015

jlukic added this to the 2.0 milestone Apr 13, 2015

jlukic added a commit that referenced this issue Apr 13, 2015

NPM Update should only update default theme folder (not whole theme p…

3d2fec4

…ath) #2101

jlukic added a commit that referenced this issue Apr 13, 2015

More obvious name for merge copy #2101

77923da

jlukic added a commit that referenced this issue Apr 13, 2015

More name consolidation #2101

588d785

jlukic modified the milestones: 1.x Changes, 2.0 Apr 13, 2015

jlukic added Confirmed Bug and removed Evaluating Bug / Change labels Apr 13, 2015

jlukic closed this as completed Apr 13, 2015

jlukic added the High Priority label Apr 13, 2015

jlukic added a commit that referenced this issue Apr 13, 2015

Install path changes #2101

d6e0ccb

jlukic added a commit that referenced this issue Apr 13, 2015

Fix theme path #2101

13303c0

jlukic added a commit that referenced this issue Apr 13, 2015

#2101 finish fix for npm install

2df4e82

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Maintain a packaged theme alongside semantic #2101

Maintain a packaged theme alongside semantic #2101

gabro commented Apr 11, 2015

jlukic commented Apr 11, 2015

gabro commented Apr 11, 2015

jlukic commented Apr 13, 2015

jlukic commented Apr 13, 2015

gabro commented Apr 13, 2015

gabro commented Apr 13, 2015

jlukic commented Apr 13, 2015

gabro commented Apr 13, 2015

jlukic commented Apr 13, 2015

jlukic commented Apr 13, 2015

gabro commented Apr 13, 2015

jlukic commented Apr 13, 2015

gabro commented Apr 13, 2015

gabro commented Apr 13, 2015

jlukic commented Apr 13, 2015

gabro commented Apr 13, 2015

jlukic commented Apr 13, 2015

gabro commented Apr 13, 2015

gabro commented Apr 13, 2015

jlukic commented Apr 13, 2015

gabro commented Apr 13, 2015

Semantic-Pusher-Robot commented Apr 13, 2015

Maintain a packaged theme alongside semantic #2101

Maintain a packaged theme alongside semantic #2101

Comments

gabro commented Apr 11, 2015

jlukic commented Apr 11, 2015

gabro commented Apr 11, 2015

jlukic commented Apr 13, 2015

jlukic commented Apr 13, 2015

gabro commented Apr 13, 2015

gabro commented Apr 13, 2015

jlukic commented Apr 13, 2015

gabro commented Apr 13, 2015

jlukic commented Apr 13, 2015

jlukic commented Apr 13, 2015

gabro commented Apr 13, 2015

jlukic commented Apr 13, 2015

gabro commented Apr 13, 2015

gabro commented Apr 13, 2015

jlukic commented Apr 13, 2015

gabro commented Apr 13, 2015

jlukic commented Apr 13, 2015

gabro commented Apr 13, 2015

gabro commented Apr 13, 2015

jlukic commented Apr 13, 2015

gabro commented Apr 13, 2015

Semantic-Pusher-Robot commented Apr 13, 2015