akanix42 / meteor-react-toolbox-example

MIT License
18 stars 1 forks source link

Support for latest Meteor and css-modules #4

Closed gadicc closed 8 years ago

gadicc commented 8 years ago

Hey @nathantreid, I thought it might be better to track all this stuff here. I'm going to close https://github.com/nathantreid/meteor-css-modules/issues/23.

Starting point, clone this repo, meteor npm install && meteor, everything works. Then:

$ meteor update

Changes to your project's package version selections from updating the release:                                              
....
meteor-react-toolbox-example: updated to Meteor 1.3.2.4.

Changes to your project's package version selections from updating package
versions:

nathantreid:css-modules                      upgraded from 1.1.0 to 1.1.1
nathantreid:css-modules-import-path-helpers  upgraded from 0.1.1 to 0.1.2

I then get the error I mentioned previously:

TypeError: Cannot call method 'substr' of undefined

so I remove from package.json:

    "explicitIncludes": [
      "node_modules/react-toolbox"
    ]

and then get:

`Error: Attempting to combine different files: client/components/style.scss` ``` Error: Attempting to combine different files: { type: 'js', sourcePath: 'client/components/style.scss', servePath: 'client/components/style.scss', installPath: 'client/components/style.scss', deps: { './style.css': {} }, lazy: true, data: , hash: '39511a6d70e7980cb52ca5519c549245aa450762' } { type: 'js', data: , sourcePath: 'client/components/style.scss', targetPath: 'client/components/style.scss', servePath: '/client/components/style.scss', hash: '99665ead3d2ee3f773ebd07ee0c0d9f6079a1c55', sourceMap: undefined, lazy: false, bare: false, mainModule: false, imported: true, installPath: 'client/components/style.scss' } at checkProperty (/tools/isobuild/import-scanner.js:274:15) at ImportScanner._combineFiles (/tools/isobuild/import-scanner.js:263:35) at /tools/isobuild/import-scanner.js:187:14 at Array.forEach (native) at ImportScanner.addInputFiles (/tools/isobuild/import-scanner.js:158:11) at /tools/isobuild/compiler-plugin.js:834:15 at Array.forEach (native) at Function.computeJsOutputFilesMap (/tools/isobuild/compiler-plugin.js:805:19) at ClientTarget._emitResources (/tools/isobuild/bundler.js:913:8) at /tools/isobuild/bundler.js:684:12 at /tools/utils/buildmessage.js:359:18 at [object Object].withValue (/tools/utils/fiber-helpers.js:89:14) at /tools/utils/buildmessage.js:352:34 at [object Object].withValue (/tools/utils/fiber-helpers.js:89:14) at /tools/utils/buildmessage.js:350:23 at [object Object].withValue (/tools/utils/fiber-helpers.js:89:14) at Object.enterJob (/tools/utils/buildmessage.js:324:26) at ClientTarget.make (/tools/isobuild/bundler.js:675:18) at /tools/isobuild/bundler.js:2474:14 at /tools/isobuild/bundler.js:2563:20 at Array.forEach (native) at Function._.each._.forEach (/home/dragon/.meteor/packages/meteor-tool/.1.3.2_4.175ert8++os.linux.x86_64+web.browser+web.cordova/mt-os.linux.x86_64/dev_bundle/lib/node_modules/underscore/underscore.js:79:11) at /tools/isobuild/bundler.js:2562:7 at /tools/utils/buildmessage.js:271:13 at [object Object].withValue (/tools/utils/fiber-helpers.js:89:14) at /tools/utils/buildmessage.js:264:29 at [object Object].withValue (/tools/utils/fiber-helpers.js:89:14) at /tools/utils/buildmessage.js:262:18 at [object Object].withValue (/tools/utils/fiber-helpers.js:89:14) at /tools/utils/buildmessage.js:253:23 at [object Object].withValue (/tools/utils/fiber-helpers.js:89:14) at Object.capture (/tools/utils/buildmessage.js:252:19) at Object.exports.bundle (/tools/isobuild/bundler.js:2455:31) at /tools/runners/run-app.js:591:36 at Function.run (/tools/tool-env/profile.js:489:12) at bundleApp (/tools/runners/run-app.js:581:34) at AppRunner._runOnce (/tools/runners/run-app.js:634:35) at AppRunner._fiber (/tools/runners/run-app.js:887:28) at /tools/runners/run-app.js:411:12 ```

Not sure what else I can try from here.

On my real project, I don't get this error. Even after removing explicitIncludes, I'm still stuck with the substr error. But I'll look at that separately.

Thanks for the help!

akanix42 commented 8 years ago

Meteor 1.3.2 changes the way css files are imported. :frowning: I've got some more digging to do, but for now you can make the example work by changing the outputJsFilePath option in package.json:

"cssModules": {
  "outputJsFilePath": "{dirname}/{basename}.js"
}

This issue affects the whole of my CSS modules plugin, see nathantreid/meteor-css-modules#24.

newswim commented 8 years ago

this is probably dumb, but @gadicc how did you make that code block collapsable? V cool.

gadicc commented 8 years ago

Thanks, @nathantreid! I got past the "attempting to combine different files" problem but then was back with to original substr (from sourceMap) crash. I just commented out this line and everything works again, didn't investigate any further yet.

Btw, what are your thoughts about ditching addStyleSheet and shipping each style sheet in it's own module via addJavaScript along with the other code you have to have there anyways? From that point I think we're just 2 steps away from hotloading support :) Only downside I can think of is missing out on some css-specific minification optimizations.

@newswim, haha, everyone (including me) has the same reaction when they see it for the first time. Read this, the "discourse expandable post" link in the next post, and the comments in the post after that.

gadicc commented 8 years ago

@nathantreid we can move this back to my original issue if it's more relevant there.

akanix42 commented 8 years ago

This is probably more relevant in the other issue, but I'm fine with just continuing it here.

I can't totally ditch addStylesheet since I actually need to have the CSS file for some of my apps, although I'm open to making it optional - the addStylesheet call is only used for non-lazy stylesheets; each lazy-loaded stylesheet is already "in it's own module via addJavaScript along with the other code", so that would be a pretty simple switch to add. The question then becomes which to make the default.

If we got hotloading working, I'd definitely turn that on for my development process!

The primary downside is if you use SSR, the styles won't load until the JS does.

Oh, and I had the same reaction to the collapsible code block you have up there!

newswim commented 8 years ago

fyi, this is how the hidden code trick is done:

<details> 
  <summary>Q1: What is the best Language in the World? </summary>
   A1: JavaScript 
</details>