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

Help upgrading to 1.x #64

Closed
sholladay opened this issue Jan 30, 2018 · 13 comments
Closed

Help upgrading to 1.x #64

sholladay opened this issue Jan 30, 2018 · 13 comments

Comments

@sholladay
Copy link

Hi and thanks for this great Rollup plugin. It's been working well for me.

I am looking to upgrade from version 0.5.6 to 1.2.7 but am having difficulty figuring out what changes I need to make.

My use case is a React app with CSS modules. The existing setup is below:

const cssExportMap = {};

postcss({
    plugins : [
        cssModules({
            getJSON(id, exportTokens) {
                cssExportMap[id] = camelcaseKeys(exportTokens);
            },
            // Rc-slider needs some global CSS classes defined
            globalModulePaths : [/node_modules\/rc-slider\/assets\/index.css/]
        })
    ],
    getExportNamed : true,
    getExport(id) {
        return cssExportMap[id];
    }
})

The main goal of this special config is to enabled named imports with the more idiomatic convention of camelcase names, avoiding the -- to $__$ conversion that this plugin does by default.

/* sheet.css */
.foo-bar { background : blue }
import { fooBar } from './sheet.css';

... much better than ...

import { foo$__$bar } from './sheet.css';

This worked perfectly prior to 1.0.0. Now I see output like below.

lib/js/components/nav-bar/nav-bar.jsx
navSearchWrapper is not exported by lib/css/nav-bar.css
21:             React.createElement(
22:                 'div',
23:                 { className: style.navSearchWrapper },
                                       ^
24:                 React.createElement('input', { name: 'search', id: 'nav-search', placeholder: 'Search' }),
25:                 React.createElement(Icon, { svgInfo: searchIcon, width: '25', height: '25' })

Based on what I see in the README now, I tried using the modules option, as below.

postcss({
    modules : {
        getJSON(id, exportTokens) {
            cssExportMap[id] = camelcaseKeys(exportTokens);
        },
        // Rc-slider needs some global CSS classes defined
        globalModulePaths : [/node_modules\/rc-slider\/assets\/index.css/]
    },
    getExportNamed : true,
    getExport(id) {
        return cssExportMap[id];
    }
})

But there was no effect, the output remains the same. Based on some logging I tried, it seems that getExport() is no longer being called.

I'm happy to migrate to any other pattern that achieves the same end goal of importing classes as camelcased names. Any advice on how to upgrade would be much appreciated.

@egoist
Copy link
Owner

egoist commented Jan 30, 2018

getExportNamed getExport no longer exist in 1.0 and above, maybe I can add an option to control how className is transformed.

@egoist
Copy link
Owner

egoist commented Jan 30, 2018

seems we can really simplify this too:

function escapeClassNameDashes(str) {
return str.replace(/-+/g, match => {
return `$${match.replace(/-/g, '_')}$`
})
}
function ensureClassName(name) {
name = escapeClassNameDashes(name)
if (reserved.check(name)) {
name = `$${name}$`
}
return name
}

it just feels weird to import class-name as class$_$name 😅

/ping @lmihaidaniel what do you think?

@egoist
Copy link
Owner

egoist commented Jan 30, 2018

Maybe simply camelcase the name and prefix with _ underscore if the final name is a reserved es keyword

@sholladay
Copy link
Author

That would certainly work for me. I didn't bother to deal with the reserved word case, as I figure it will simply break right away and the developer will just change the class name. But I guess being able to import { _class } from './sheet.css' could be useful in some corner case.

@egoist egoist closed this as completed in 3b6d7cb Jan 30, 2018
@egoist
Copy link
Owner

egoist commented Jan 30, 2018

See https://github.com/egoist/rollup-plugin-postcss#namedexports it can be a function now in v1.2.8.

We might simplify the default havior in v2.0 since it's a breaking change.

@mosanger
Copy link
Contributor

Thanks for the change, I was waiting for that.
While generated names are is in the named exports now, they are missing in the default export, which was working before.

so if you only do
import styles from './styles.scss';
styles.myName does not exist.

import styles, { myName } from './styles.scss
does work though.

@sholladay
Copy link
Author

You could do this to get all classes:

import * as styles from './styles.scss';

@mosanger
Copy link
Contributor

mosanger commented Feb 16, 2018

yeah, thanks, I already did that for now...
still thought it would be good to have them in the default export, like it was before (pre 1.0.0)

@egoist
Copy link
Owner

egoist commented Feb 16, 2018

hmm it should be in default export too, otherwise there must be something wrong in the code..

@mosanger
Copy link
Contributor

I fixed the issue in a fork, would you accept a pull request?

@egoist
Copy link
Owner

egoist commented Mar 29, 2018

@mosanger fixed what?

@mosanger
Copy link
Contributor

Named exports not being in the default export
mosanger@a3020b6

@egoist
Copy link
Owner

egoist commented Mar 29, 2018

@mosanger OK, LGTM, PR welcome 😄

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

No branches or pull requests

3 participants