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(web): optimized rollup output logic #8526

Closed
wants to merge 1 commit into from
Closed

fix(web): optimized rollup output logic #8526

wants to merge 1 commit into from

Conversation

xiejay97
Copy link
Contributor

ISSUES CLOSED: #8475
ISSUES CLOSED: #8505

Current Behavior

Expected Behavior

Related Issue(s)

Fixes #

@nx-cloud
Copy link

nx-cloud bot commented Jan 14, 2022

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 9d66981. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 13 targets

Sent with 💌 from NxCloud.

@vercel
Copy link

vercel bot commented Jan 14, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/nrwl/nx-dev/7d8Z7jgmZv1NVTm12BTK7eaS54xr
✅ Preview: Canceled

[Deployment for ee31965 canceled]

packages/web/src/executors/rollup/rollup.impl.ts Outdated Show resolved Hide resolved
packages/web/src/executors/rollup/rollup.impl.ts Outdated Show resolved Hide resolved
@AgentEnder
Copy link
Member

@xiejay97 Do you care to run nx format and re-push your branch? If we can get CI green on this, it would be good to get merged.

@AgentEnder
Copy link
Member

Fixes #9817

ISSUES CLOSED: #8475
ISSUES CLOSED: #8505
@vercel
Copy link

vercel bot commented Apr 22, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Updated
nx-dev ⬜️ Ignored (Inspect) Apr 22, 2022 at 7:14AM (UTC)

@xiejay97
Copy link
Contributor Author

@xiejay97 Do you care to run nx format and re-push your branch? If we can get CI green on this, it would be good to get merged.

OK now

@AndyClausen
Copy link
Contributor

One thing to note here:
If I have a package A with tests that depend on package B, I need to set the main field in package B to index.ts or the tests in pakcage A will fail.
After this PR is merged, if I set the main field, it won't be overwritten when running rollup and I will be "unknowingly" publishing with the incorrect main field.
Maybe this is how it should work though, and the problem lies with Jest. I thought it was worth mentioning nonetheless.

@jaysoo
Copy link
Member

jaysoo commented May 30, 2022

This is fixed in latest Nx version (14.1.9)

@jaysoo jaysoo closed this May 30, 2022
@github-actions
Copy link

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generated package.json should honour main and module fields @nrwl/web:rollup produces the wrong output
4 participants