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

experimental.css not support legacy browser? #16147

Open
hardfist opened this issue Aug 15, 2022 · 22 comments
Open

experimental.css not support legacy browser? #16147

hardfist opened this issue Aug 15, 2022 · 22 comments

Comments

@hardfist
Copy link
Contributor

Bug report

What is the current behavior?
experimental.css may not supports legacy browser because it relys on css variable to load css chunk, I'm wondering whether it's by design

/******/ 		var loadCssChunkData = (target, link, chunkId) => {
/******/ 			var data, token = "", token2, exports = {}, exportsWithId = [], exportsWithDashes = [], i = 0, cc = 1;
/******/ 			try { if(!link) link = loadStylesheet(chunkId); data = link.sheet.cssRules; data = data[data.length - 1].style; } catch(e) { data = getComputedStyle(document.head); }
/******/ 			data = data.getPropertyValue("--webpack-" + uniqueName + "-" + chunkId);
/******/ 			if(!data) return [];
/******/ 			for(; cc; i++) {
/******/ 				cc = data.charCodeAt(i);
/******/ 				if(cc == 40) { token2 = token; token = ""; }
/******/ 				else if(cc == 41) { exports[token2.replace(/^_/, "")] = token.replace(/^_/, ""); token = ""; }
/******/ 				else if(cc == 47 || cc == 37) { token = token.replace(/^_/, ""); exports[token] = token; exportsWithId.push(token); if(cc == 37) exportsWithDashes.push(token); token = ""; }
/******/ 				else if(!cc || cc == 44) { token = token.replace(/^_/, ""); exportsWithId.forEach((x) => (exports[x] = uniqueName + "-" + token + "-" + exports[x])); exportsWithDashes.forEach((x) => (exports[x] = "--" + exports[x])); __webpack_require__.r(exports); target[token] = ((exports, module) => {
/******/ 					module.exports = exports;
/******/ 				}).bind(null, exports); token = ""; exports = {}; exportsWithId.length = 0; }
/******/ 				else if(cc == 92) { token += data[++i] }
/******/ 				else { token += data[i]; }
/******/ 			}
/******/ 			installedChunks[chunkId] = 0;
/******/ 		
/******/ 		}

If the current behavior is a bug, please provide the steps to reproduce.

What is the expected behavior?

Other relevant information:
webpack version:
Node.js version:
Operating System:
Additional tools:

@alexander-akait
Copy link
Member

Yes, do you need to support old browsers?

@hardfist
Copy link
Contributor Author

yes, we still need to support old browsers like browser in android 4.4-

@alexander-akait
Copy link
Member

oh, in thsi case css-loader can help here, new approach has better perf and logic, but requires new browsers

@webpack-bot
Copy link
Contributor

This issue had no activity for at least three months.

It's subject to automatic issue closing if there is no activity in the next 15 days.

@alexander-akait
Copy link
Member

bump

@webpack-bot
Copy link
Contributor

Issue was closed because of inactivity.

If you think this is still a valid issue, please file a new issue with additional information.

@alexander-akait
Copy link
Member

@TheLarkInn Maybe we should mark this in docs as non fixable and required modern browsers

@hardfist
Copy link
Contributor Author

hardfist commented Jun 8, 2023

@TheLarkInn Maybe we should mark this in docs as non fixable and required modern browsers

I think this is very important, actually when I finish migrating my tools from css-loader to experiments.css and find the compatible problem I feel depressed

@alexander-akait
Copy link
Member

alexander-akait commented Jun 8, 2023

@hardfist

I think this is very important, actually when I finish migrating my tools from css-loader to experiments.css and find the compatible problem I feel depressed

CSS will work on old browsers ("production"), only dev mode and HMR will not work on old browsers

@hardfist
Copy link
Contributor Author

hardfist commented Jun 8, 2023

CSS will work on old browsers ("production"), only dev mode and HMR will not work on old browsers

glad to know this, because we do have lots of application users who need to use legacy browser(you know the old android machine), so rspack differs with webpack now in css implementation for sake of legacy browsers, so if webpack could support legacy browsers(even only in production), we can align to this webpack's behavior now.

@alexander-akait
Copy link
Member

@hardfist I will add testcases to ensure it, but based on code it should work, let's look at our code, we try to use link.sheet.cssRules and if nothing we try to use getComputedStyle - https://developer.mozilla.org/ru/docs/Web/API/Window/getComputedStyle, but yeah it can be a problem for WebView Android v4.3, on theory we can try to use el.currentStyle and for ie8, there are two solutions

  • recommend to polyfill it
  • try to find another workaround, for very old browsers

But I think it is fine with recommend to polyfill it if workaround will be big or have a bad perf

@alexander-akait
Copy link
Member

I did some research and fixes (I think we will fix it in multiple PRs, the first will be soon):

So for the last we can:

  • Try to parse all links using cssText/currentScript and parse text on CSS variables using regexp, it will be slow
  • Use ajax and using regexp try to extract modules - will be very slow, I don't like it
  • Move some logic on HTML level, i.e. developer put <link rel="stylesheet" data-webpack="webpack:chunk-main" href="./main.css" data-webpack-modules="\.\/src\/foo\.css,\.\/src\/style\.css;"> (it can be done/automated using a plugin), webpack in runtime try to use getComputedStyle, if nothing was found we will try to get this meta tag and get the chunk name. But here is an limitation - it will work only for initial chunks... But we can use meta for async chunks. But it can bloat the source code of HTML page, especial for very big applications.

Our main problem is that we need to understand where and how to store modules which concated into chunk.

I am now downloading images of old systems to try different approaches. Maybe I can find some other approaches and solutions.

@alexander-akait
Copy link
Member

Technically, of course, we can try to store all modules (for initial and async) inside JS runtime chunk, this will increase the size of runtime chunk. On the other hand, we can only do this when your don't have css variables support in your browserslist and run it as fallback if can't get value using getComputedStyle

@hyf0
Copy link
Contributor

hyf0 commented Jun 28, 2023

we can only do this when your don't have css variables support in your browserslist and run it as fallback if can't get value using getComputedStyle

I think this is reasonable.

Technically, of course, we can try to store all modules (for initial and async) inside JS runtime chunk

Do we really need to support legacy browser in this way?

I thought we just need to create facade .js files like this https://github.com/ICJR/rspack-repro/blob/525f0cb8c150307bda33f010adeda27d07b9303f/rspack-dist/something/else.js at web-infra-dev/rspack#3608.

If I remember correctly, this is also how Webpack bundles CSS with targeting node.

@hyf0
Copy link
Contributor

hyf0 commented Jun 28, 2023

we can only do this when your don't have css variables support in your browserslist

Let me try to describe the behaviors. @alexander-akait Please correct me if I'm wrong.

If the user set target: browserslist, the bundler would choose the correct way to bundle CSS based on .browserlistrc.

If the user set target: web, the bundler would choose the CSS variable way to bundle CSS.

@ScriptedAlchemy
Copy link
Member

bump

@hardfist
Copy link
Contributor Author

@alexander-akait it seems it is not resolved can we reopen it

@alexander-akait
Copy link
Member

@hardfist Yeah, can you describe which browsers do you want to support, or just any old browser?

@hardfist
Copy link
Contributor Author

@hardfist Yeah, can you describe which browsers do you want to support, or just any old browser?

thanks, the lowest version we have to support is iOS 10 & android 5

@JSerFeng
Copy link
Contributor

Can we store the css modules name mapping at JS chunk instead of css chunk, the downside is that it will increase the size of the JS chunk, use runtime module to register css modules after loading a chunk

@JSerFeng
Copy link
Contributor

it will increase the size of the JS chunk

I think this is okay because the total size should be the same.
Current strategy, when loading a chunk, total size is: js + (css + css meta)
If put css meta in js chunk: total size: (js + css meta) + css

@alexander-akait
Copy link
Member

We can try it and check it out

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants