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: watch using rollup v4 #1670

Merged
merged 1 commit into from
Jan 23, 2025
Merged

fix: watch using rollup v4 #1670

merged 1 commit into from
Jan 23, 2025

Conversation

cbt1
Copy link
Contributor

@cbt1 cbt1 commented Jan 22, 2025

Motivation

Fixes #1471 , the issue where the watch flag in the build command does not pick up file changes. This also includes the serve command since it's using the same underlying code.

The root cause of this issue seem to be that the rollup-plugin-sourcemaps plugin has not updated to support Rollup v4, which has some changes to it's Plugin API (https://rollupjs.org/migration/#changes-to-the-plugin-api).

From the tests I've done Babel source maps works fine as a replacement.

There still seem to be some issue related to @rollup/plugin-commonjs where it can crash the build/watch process. But I'm not able to reproduce that consistently across the different nebula.js projects I've tested with. But this PR rollup/plugins#1639 seem to be the culprint of that crash at least. So using v26.0.3 should still work in those projects.

Requirements checklist

  • Api specification
    • Ran yarn spec
      • No changes OR API changes has been formally approved
  • Unit/Component test coverage
  • Correct PR title for the changes (fix, chore, feat)

When build and tests have passed:

  • Add code reviewers, for example @qlik-oss/nebula-core

@cbt1 cbt1 marked this pull request as ready for review January 22, 2025 14:34
@cbt1 cbt1 requested review from Caele, T-Wizard and a team January 22, 2025 14:34
@T-Wizard
Copy link
Collaborator

From the tests I've done Babel source maps works fine as a replacement.

have you tested that source maps from dependencies are loaded? and not just source map for the chart itself

@cbt1
Copy link
Contributor Author

cbt1 commented Jan 23, 2025

From the tests I've done Babel source maps works fine as a replacement.

have you tested that source maps from dependencies are loaded? and not just source map for the chart itself

Just did some testing and it appears that it fails to load the sourcemaps for dependencies. For now I think we have to live with that but ideally it should be solved as well.

@cbt1 cbt1 merged commit 9a17c9b into main Jan 23, 2025
11 checks passed
@cbt1 cbt1 deleted the cbt/fix/watch-command branch January 23, 2025 11:51
@T-Wizard
Copy link
Collaborator

From the tests I've done Babel source maps works fine as a replacement.

have you tested that source maps from dependencies are loaded? and not just source map for the chart itself

Just did some testing and it appears that it fails to load the sourcemaps for dependencies. For now I think we have to live with that but ideally it should be solved as well.

we could try using rollup-plugin-sourcemaps2
https://www.npmjs.com/package/rollup-plugin-sourcemaps2
https://github.com/2wce/rollup-plugin-sourcemaps2
that should be a version rollup-plugin-sourcemaps updated to works with rollup 4.x.x or later.

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.

nebula serve does not refresh visualisation/extension when changes are made to /src/index.js
2 participants