CJS / ESM selection case - styled-components

guybedford commented 2 years ago

Styled components broke recently at https://generator.jspm.io/#U2NhYGBkDM0rySzJSU1hKC6pBFK6yfm5Bfl5qXklxQ6mesZ6xgCnze/3JgA.

This is due to the CJS variant being used from https://ga.jspm.io/npm:@emotion/stylis@0.8.5/dist/stylis.browser.cjs.js instead of the .esm.js variant.

With the dominance change in https://github.com/jspm/generator/pull/86, issues like this might be more likely.

Steps:

[ ] Test the latest version of this generator source against it to see if it's still doing that.
[ ] Try changing the priority in the PR at https://github.com/jspm/generator/pull/86/files to instead do a !wasCJS variant on all the conditionals to check if the ESM priority can be made
[ ] Possibly consider allowing priority to be user-defined to resolve cases like this in future or alternative customization approaches.

guybedford commented 2 years ago

@JayaKrishnaNamburu finally tracked this one down and it wasn't due to the determinism issue, but a previously resolved CDN bug around "module" and "browser" interaction.

JayaKrishnaNamburu commented 2 years ago

Thanks for looking into this @guybedford. https://codesandbox.io/s/serene-neumann-07flq?file=/module.js Looks like we have similar issue around @emotion/is-prop-valid which doesn't have export maps 😓

JayaKrishnaNamburu commented 2 years ago

We are trying to introduce import-map workflows for users and so trying to put together a template with jspm cdn and our esm components which is using styled-components

guybedford commented 2 years ago

@JayaKrishnaNamburu if you look at the package.json for is-prop-valid - https://ga.jspm.io/npm:@emotion/is-prop-valid@1.1.1/package.json you can see it has the correct branching for the browser both for browser + module and browser + !module condition paths.

Looking at your sandbox it's because you're on an older cached build of is-prop-valid - https://ga.jspm.io/npm:@emotion/is-prop-valid@0.8.8/package.json that doesn't have that in the package.json.

I can rebuild this 0.8.8 version for you.

guybedford commented 2 years ago

Ok, the cache clear should complete shortly, then it should be working. Just let me know if there are any other package + versions you need. I trust you can tell from the package.json what to look for by now.

JayaKrishnaNamburu commented 2 years ago

That would be pretty helpful 😄

JayaKrishnaNamburu commented 2 years ago

And i believe this map is generated by jspm itself right. Because i don't see any on the emotion package itself. https://unpkg.com/browse/@emotion/is-prop-valid@1.1.1/package.json

Would it still e a good thing having on the package itself, because i am trying to make a PR if so, to add exports to emotion packages 😄

guybedford commented 2 years ago

@JayaKrishnaNamburu yes JSPM automatically generates the "exports" field for all packages. It's obviously better for packages to define their own of course, but it's perfectly fine to support "module" and "browser" fields, there was just a bug only fixed recently in the conversion that wasn't supporting the combination of "module" and "browser" together, a pattern emotion uses extensively.

JayaKrishnaNamburu commented 2 years ago

Ah got it, got it 👍

JayaKrishnaNamburu commented 2 years ago

Here is a working example, needed a little bit of manual correction of import-map https://codepen.io/jkrishna/pen/VwMaWaN?editors=1010

Import map -> https://generator.jspm.io/#U2NjYGBkDM0rySzJSU1hKC6pBFK6yfm5Bfl5qXklxQ6mesZ6xgDY5X4tJgA

@emotion/stylis is pointing to esm but the other two emotion packages @emotion/is-prop-valid and @emotion/memonize still pointing to cjs paths. A manual change of it, and it started working. Thanks for helping with this one 😄

guybedford commented 2 years ago

@JayaKrishnaNamburu rebuilt and flushed those caches. Should be working now with a cache clear!

jspm / generator

CJS / ESM selection case - styled-components #89