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

Fix @tailwindcss/line-clamp warning #10919

Merged
merged 8 commits into from
Mar 30, 2023
Merged

Conversation

thecrypticace
Copy link
Contributor

Previously the check was in the resolveConfig path which is importable externally by users because it's public API. However, due to build tools statically hoisting conditional imports (cough Rollup) we had to back that out.

Moving this to validateConfig gives us the same flexibility we had previously but now happens at build-time only. It is not public API and therefore can use dynamic requires.

While working on this we also encountered an issue with the process variable being accessed in the browser when using resolveConfig and have fixed this as well.

Fixes #10894
Fixes #10918

RobinMalfait and others added 5 commits March 30, 2023 19:25
This only happens in setupTrackingContext outside of resolveConfig
The important thing is that this happens in Node-land only. It is outside of `resolveConfig` which is public and importable into user projects. That is the scenario that breaks because of static import hoisting.
The `resolveConfig` dep path is public which should not reference process. However, we have some behavior that changes based on env vars so we need to conditionalize it instead.
@thecrypticace thecrypticace merged commit 50174f9 into 3.3 Mar 30, 2023
@thecrypticace thecrypticace deleted the fix/warn-based-on-package-json branch March 30, 2023 19:06
thecrypticace added a commit that referenced this pull request Mar 30, 2023
* WIP

* Move warning to validateConfig

This only happens in setupTrackingContext outside of resolveConfig

* Use original dynamic require approach in `validateConfig`

The important thing is that this happens in Node-land only. It is outside of `resolveConfig` which is public and importable into user projects. That is the scenario that breaks because of static import hoisting.

* Don’t reference process when it might be undefined

The `resolveConfig` dep path is public which should not reference process. However, we have some behavior that changes based on env vars so we need to conditionalize it instead.

* Update changelog

* Formatting

* More formatting

* Update changelog

---------

Co-authored-by: Robin Malfait <malfait.robin@gmail.com>
Co-authored-by: Jonathan Reinink <jonathan@reinink.ca>
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

Successfully merging this pull request may close these issues.

None yet

3 participants