Need better way to determine componentId about babel-plugin-styled-components HOT 20 CLOSED

Maybe it's better to try again to find a way to make it work without componentId in SSR.

from babel-plugin-styled-components.

Wait why is this breaking? What's going on here?

from babel-plugin-styled-components.

On the first run in the development mode babel-loader compiles all the files and puts in the cache. Cache is stored in /node_modules/.cache/react-scripts.

Interrupt execution and restart npm start. Server starts without compilation with data from cache. Now, if you change any application file, only this file will be recompiled with id = 0. I've put id to display name to illustrate it.

from babel-plugin-styled-components.

Oh god that's super annoying. 😕 /cc @geelen

from babel-plugin-styled-components.

hahah that's awesome. I knew incremental IDs were going to break down, I just hoped we'd get a bit of mileage out of them first :)

For other options we'll have ask user to configure babel to provide sourceRoot and fileName

What does that look like? If we append code and filename together then hash them we should be ok, right?

Alternatively, for dev, we use something like a timestamp instead of a sequential ID. A componentId only has to be deterministic for SSR. Does create-react-app use the babel cache for production build as well, or just dev?

from babel-plugin-styled-components.

For babel-loader I'm getting absolute fileName in tests. For babel.transformFile(pathToFile) and babel cli there is fileName in a form it was passed to the babel. I guess, in most cases we will have some fileName to depend on. Absolute or relative to the project root.

There will be no fileName if it's used with babel.transform(source, options). It can be passed in the options. babel.transform(source, {filename})

Another problem with not deterministic component Ids in production is that it will make chunkhash change on every build.

Does create-react-app use the babel cache for production build as well, or just dev?

No, it's not.

from babel-plugin-styled-components.

I've been thinking about this for a bit... Sources we can't use are:

• incremental ids (breaks with caching)
• filenames (might not exist with babel.transform)

Sources we could use are:

• the identifier name (not necessarily unique; sometimes there isn't one 😢)
• hash of the CSS code
• hash of the file's code
• timestamp

Am I correct when assuming that we only need a unique id for the lifetime of that component? We just need a unique classname for the bundle, and when a component changes, it's okay for that classname to change.

If that holds up we might as well use a time-based unique id, like shortids, and just attach the identifier to it, if we have one, for human-readable classnames.

from babel-plugin-styled-components.

@mxstbr @vdanchenkov @geelen I quickly drafted up a PR which uses shortids, just to see what it would look like ;)

#27

from babel-plugin-styled-components.

Hmm, definitely worth a try! I'm not 100% sure that a random ID won't break something eventually (I'd hoped for a deterministic one) but I can't think of anything at the moment, so I reckon that's good. I'll let @mxstbr review the PR tho, since i have no idea how babel transforms work :)

from babel-plugin-styled-components.

@geelen I hope it's fine. I think it's the lazy, but sure-fire way out of this dilemma.

Oh, and fear not- actually no one knows how babel transforms work 🥇

from babel-plugin-styled-components.

One concern I have about the time based approach is that the non detminism could bite you when you have more than one bundle. For example, I generate a build for the browser with lazy loaded routes and a non-lazy build for the server (for SSR) . They are just two different entry points in the same webpack config but I'm worried that because of this, the class names wouldn't match up.

P.S. I sincerely appreciate the work that has gone into this project!

from babel-plugin-styled-components.

@philpl Another problem of random ids is that content hashes will change on every build.

from babel-plugin-styled-components.

@nicksrandall @vdanchenkov Hm, well before the contentId would change, as well as id was not deterministic and depended on the build-order ^^

yea, that's true. the hashing the entire bundle to check for changes will become entirely useless.

we can still try to just hash everything that is being passed to styled, which should work as long as we don't have two styled calls with the exact same content, babel nodes, and display name...

from babel-plugin-styled-components.

I think that might be a better approach. Make sure to include fileName if it's present as well.

Thanks for your efforts, I say we ship something and get some real-world data on where it breaks down. I don't think changing the ID generation strategy should constitute a breaking change, so it should be fine to iterate on until we get it right.

from babel-plugin-styled-components.

I'm going to make componentId based on:

1. package name
2. path to the file from the package root folder with separators converted to forward slashes.
3. file source code
4. styled component index inside the file.

Hope it will make id deterministic across build environments.

from babel-plugin-styled-components.

Also, I'm going to drop displayName from component id. It can be added back in the runtime from the displayName field if needed.

from babel-plugin-styled-components.

Also, I'm going to drop displayName from component id. It can be added back in the runtime from the displayName field if needed.

Why? We want to have the component name in the className?

from babel-plugin-styled-components.

@mxstbr Benefit is that it makes JS bundle smaller. Makes smaller CSS generated in SSR. In the production you usually don't need to keep human-readable name. They are harder to compress compared to hashes in my proposal.

from babel-plugin-styled-components.

I think that component display name in className is a good thing. But I don't think that we need to keep a duplication to achieve that.

In development mode with everything enabled we'll have

{
    displayName: "WrappedComponent",
    componentId: "kam0ab-1"
}
// instead of
{
    displayName: "WrappedComponent",
    componentId: "WrappedComponent-kam0ab-1"
}

But className="WrappedComponent-kam0ab-1" can be easily reconstructed at runtime.

In the production with displayName: false plugin will generate

{
    componentId: "kam0ab-1"
}

from babel-plugin-styled-components.

can be easily reconstructed at runtime.

You're probably right, we should do that at runtime. PR against the v2 branch? 😉

from babel-plugin-styled-components.