Skip to content

Made the initial build for npm - #25

Merged
nikoscham merged 15 commits into
devfrom
npm-release
May 22, 2025
Merged

Made the initial build for npm#25
nikoscham merged 15 commits into
devfrom
npm-release

Conversation

@sridhar-mani

Copy link
Copy Markdown
Collaborator

Implemented rollup and initial built for the library to be published in npm.

@nikoscham
nikoscham changed the base branch from main to devMay 22, 2025 05:10
@nikoscham
nikoscham requested a review from CopilotMay 22, 2025 05:10

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR establishes the initial npm build setup by integrating Rollup to bundle the library, updating the package metadata, and configuring npm publication.

  • Introduces a Rollup configuration to produce multiple module formats (cjs, esm, umd).
  • Sets up package.json and an .npmignore file as part of the initial npm build configuration.

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

FileDescription
rollup.config.jsConfigures Rollup with plugins for bundling, resolving, and minifying the library.
package.jsonDefines the npm package metadata and scripts including the build process.
.npmignoreLists files and directories to be ignored during npm publication.
Comments suppressed due to low confidence (1)

.npmignore:6

  • Excluding the 'dist/' folder in .npmignore might prevent the built artifacts from being published to npm. Consider removing 'dist/' from .npmignore if the intent is to distribute the build outputs.
dist/

Comment threadpackage.json Outdated
… type
- Change license from AGPLv3 to MIT to match actual project license
- Replace HTML image tag with proper text description
- Update type to "module" to align with ES module syntax
@nikoscham
nikoscham requested a review from CopilotMay 22, 2025 06:50

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR sets up the initial build and packaging pipeline for publishing the library to npm.

  • Adds a VERSION export in the main entry point
  • Introduces rollup.config.js with plugins for bundling, TypeScript, and WASM support
  • Updates package.json and .npmignore for npm publishing

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

FileDescription
src/index.jsExported VERSION constant to reflect package version
rollup.config.jsConfigured Rollup outputs (CJS, ESM, UMD) and plugins
package.jsonDefined package metadata, build scripts, dependencies, and exports
.npmignoreExcluded non-published directories and files
Comments suppressed due to low confidence (1)

package.json:23

  • [nitpick] The default test script currently fails; consider adding a placeholder test suite or updating the script to run real tests.
"test": "echo \"Error: no test specified\" && exit 1"

Comment threadsrc/index.js
Comment threadrollup.config.js Outdated
Comment threadpackage.json Outdated
sridhar-maniand others added 2 commits May 22, 2025 12:28
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@nikoscham
nikoscham requested a review from CopilotMay 22, 2025 07:01

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Initial setup for npm publishing: adds version export, Rollup build config, and package metadata.

  • Exposes a VERSION constant in the library entry point.
  • Configures Rollup to build CJS, ESM, and UMD bundles with TypeScript, WASM support, and minification.
  • Creates package.json with publish scripts, dependencies, and .npmignore exclusions.

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

FileDescription
src/index.jsExported VERSION constant
rollup.config.jsAdded Rollup configuration for builds
package.jsonInitial package metadata and scripts
.npmignoreDefined files/directories to exclude
Comments suppressed due to low confidence (2)

package.json:23

  • The test script currently always exits with an error. Adding a real test command or configuring a test framework will ensure CI stability and coverage.
"test": "echo \"Error: no test specified\" && exit 1"

.npmignore:1

  • [nitpick] You’ve excluded the examples/ directory but declared it in package.json’s directories. If examples should be published as docs, remove it from .npmignore.
examples/

Comment threadrollup.config.js Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@FEAScriptFEAScript deleted a comment from CopilotAIMay 22, 2025
@nikoscham
nikoscham requested a review from CopilotMay 22, 2025 07:16

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR sets up the initial build configuration for publishing the library to npm by implementing rollup.

  • Added an export for the library version in the source code.
  • Introduced a complete rollup configuration and adjusted package.json and .npmignore files accordingly.

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.

FileDescription
src/index.jsAdded a version export to the library interface.
rollup.config.jsAdded initial build configuration with plugins.
package.jsonConfigured project metadata and dependencies.
.npmignoreDefined files and directories to ignore when publishing.
Comments suppressed due to low confidence (1)

rollup.config.js:38

  • MAX_WASM_FILE_SIZE is used without being defined. Please define this variable or replace it with a specific numeric value to avoid runtime issues.
maxFileSize: MAX_WASM_FILE_SIZE,

@nikoscham
nikoscham requested a review from CopilotMay 22, 2025 07:24

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Implements an initial Rollup build and npm packaging for the library.

  • Adds a VERSION export to the entry point
  • Introduces rollup.config.js with CJS, ESM, and UMD bundles
  • Configures package.json and .npmignore for publishing

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

FileDescription
src/index.jsExports a VERSION constant
rollup.config.jsDefines build inputs, outputs, and plugins
package.jsonSets package metadata, scripts, dependencies, exports
.npmignoreExcludes examples, tests, and lockfile from package
Comments suppressed due to low confidence (1)

package.json:57

  • The rollup-plugin-wasm dependency is not referenced in rollup.config.js. Remove it if it's unused to reduce install size and maintenance overhead.
"rollup-plugin-wasm": "^3.0.0",

Comment threadsrc/index.js
export { logSystem, printVersion } from "./utilities/loggingScript.js";
export { plotSolution } from "./visualization/plotSolutionScript.js";
export { FEAScriptWorker } from "./workers/workerScript.js";
export const VERSION = "0.1.1"; No newline at end of file

CopilotAIMay 22, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The version string is duplicated in both code and package.json. Consider importing it from package.json or injecting it at build time to avoid drift.

Copilot uses AI. Check for mistakes.
@nikoscham
nikoscham requested a review from CopilotMay 22, 2025 07:30

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Initial setup for npm distribution, including build configuration and version export.

  • Added VERSION constant to the library entry point
  • Introduced Rollup configuration for CJS, ESM, and UMD builds
  • Configured package.json for publishing and .npmignore to exclude unwanted files

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

FileDescription
src/index.jsExported a VERSION constant matching package version
rollup.config.jsAdded Rollup config with plugins and external definitions
package.jsonDefined name, version, exports, scripts, and dependencies
.npmignoreExcluded examples, tests, and lockfile from package
Comments suppressed due to low confidence (1)

package.json:23

  • A test script is defined but there are no test files in the repo; consider adding an initial test suite or removing the script until tests are in place.
"test": "jest"

Comment threadsrc/index.js
export { logSystem, printVersion } from "./utilities/loggingScript.js";
export { plotSolution } from "./visualization/plotSolutionScript.js";
export { FEAScriptWorker } from "./workers/workerScript.js";
export const VERSION = "0.1.1"; No newline at end of file

CopilotAIMay 22, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maintaining the version in both package.json and source can lead to mismatches; consider importing the version from package.json or using a build replacement plugin to keep them in sync.

Copilot uses AI. Check for mistakes.
Comment threadsrc/index.js
@@ -13,3 +13,4 @@ export { importGmshQuadTri } from "./readers/gmshReaderScript.js";
export { logSystem, printVersion } from "./utilities/loggingScript.js";
export { plotSolution } from "./visualization/plotSolutionScript.js";
export { FEAScriptWorker } from "./workers/workerScript.js";

CopilotAIMay 22, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] It might be helpful to add a brief JSDoc comment for the VERSION export so consumers understand its purpose and format.

Suggested change
export{FEAScriptWorker}from"./workers/workerScript.js";
export{FEAScriptWorker}from"./workers/workerScript.js";
/**
*Representsthecurrentversionofthelibrary.
*Followssemanticversioning: MAJOR.MINOR.PATCH.
*/

Copilot uses AI. Check for mistakes.
@nikoscham
nikoscham requested a review from CopilotMay 22, 2025 07:54

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Initial setup for publishing the library to npm, including build tooling, package metadata, and version export.

  • Added Rollup build configuration to output CJS, ESM, and UMD bundles with source maps and minification.
  • Updated package.json with entry points, scripts, dependencies, and version.
  • Exported a VERSION constant and configured .npmignore for packaging.

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

FileDescription
src/index.jsExported VERSION constant to expose package version at runtime
rollup.config.jsNew Rollup config defining input, outputs, plugins, externals
package.jsonPopulated fields (name, version, scripts, exports, peerDependencies)
.npmignoreExcluded examples, tests, lockfile, and gitignore from package
Comments suppressed due to low confidence (1)

package.json:23

  • No tests are configured yet, leaving core functionality unverified. Consider adding a basic test suite to validate builds and API behavior.
"test": "echo \"No tests configured\" && exit 0"

Comment threadsrc/index.js
export { logSystem, printVersion } from "./utilities/loggingScript.js";
export { plotSolution } from "./visualization/plotSolutionScript.js";
export { FEAScriptWorker } from "./workers/workerScript.js";
export const VERSION = "0.1.1"; No newline at end of file

CopilotAIMay 22, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hard-coding the version in code requires manual sync with package.json. Consider deriving the version from package.json at build time to avoid mismatches.

Copilot uses AI. Check for mistakes.
Comment threadrollup.config.js
name: "FEAScript",
sourcemap: true,
globals: {
mathjs: "math",

CopilotAIMay 22, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The UMD global for mathjs is typically mathjs, not math. This mismatch can cause mathjs imports to be undefined in browser builds.

Suggested change
mathjs: "math",
mathjs: "mathjs",

Copilot uses AI. Check for mistakes.
@nikoscham
nikoscham requested a review from CopilotMay 22, 2025 08:32

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR sets up the initial npm build by configuring Rollup, updating package metadata, exporting a version constant, and adding installation docs.

  • Added Rollup config for CJS, ESM, and UMD bundles with source maps and minification
  • Updated package.json with entry points, scripts, peer dependencies, and version bump
  • Exported VERSION constant and expanded README with installation options

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
FileDescription
src/index.jsExported VERSION constant for runtime version check
rollup.config.jsIntroduced Rollup build configuration and plugins
package.jsonAdded build scripts, entry points, and metadata
README.mdExpanded installation instructions and usage examples
.npmignoreDefined files/folders to exclude from npm package
Comments suppressed due to low confidence (3)

package.json:23

  • There are no tests configured yet. Adding basic tests for your build outputs and core functionality will help catch regressions as the library evolves.
"test": "echo \"No tests configured\" && exit 0"

rollup.config.js:10

  • [nitpick] Using a .cjs.js extension is unconventional. Consider naming your CommonJS bundle with a single .cjs extension (e.g., dist/feascript.cjs) for clarity.
file: "dist/feascript.cjs.js",

rollup.config.js:1

  • [nitpick] The TypeScript plugin is configured but the input is a .js file. If you have no .ts sources, you can remove this plugin to simplify the build process.
import typescript from "rollup-plugin-typescript2";

Comment threadREADME.md Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@FEAScriptFEAScript deleted a comment from CopilotAIMay 22, 2025
@nikoscham
nikoscham requested a review from CopilotMay 22, 2025 09:49

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request introduces the initial build configuration for npm by adding the rollup build setup, a version constant export, and updating project metadata and installation instructions.

  • Added a version constant export in src/index.js
  • Introduced a rollup configuration to generate CommonJS, ESM, and UMD builds
  • Updated package metadata and README installation instructions to reflect npm publication

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
src/index.jsAdded a VERSION constant for tracking the library version.
rollup.config.jsConfigured build outputs and plugins for bundling the library.
package.jsonSet package metadata including version, module formats, etc.
README.mdUpdated installation and usage instructions for FEAScript.
.npmignoreDefined files/folders to exclude from the npm package.

Comment threadREADME.md Outdated

// Apply boundary conditions
model.addBoundaryCondition("boundaryIndex", ["conditionType", /* parameters */]);
model.addBoundaryCondition("boundaryIndex", ["conditionType" /* parameters */]);

CopilotAIMay 22, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removal of the comma in the array literal may lead to copying errors if multiple parameters are intended. Please consider reintroducing the comma to clearly indicate that the array should contain more than one element.

Suggested change
model.addBoundaryCondition("boundaryIndex", ["conditionType" /* parameters */]);
model.addBoundaryCondition("boundaryIndex", ["conditionType", /* parameters */]);

Copilot uses AI. Check for mistakes.

@nikoschamnikoscham left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to be working. I will upload it to npm. Thanks!

@nikoscham
nikoscham merged commit b494dd5 into devMay 22, 2025
@sridhar-mani

sridhar-mani commented May 22, 2025

Copy link
Copy Markdown
CollaboratorAuthor

Sure can you add me as a contributor there. Same username - https://www.npmjs.com/~sridhar-mani

@nikoscham

Copy link
Copy Markdown
Member

Sure can you add me as a contributor there. Same username - https://www.npmjs.com/~sridhar-mani

Published in https://www.npmjs.com/package/feascript :-)

I have also added you here (package.json):

"contributors": [
{
"name": "sridhar-mani",
"url": "https://www.npmjs.com/~sridhar-mani"
}
],

I cannot see, however, your username displayed in the collaborators here: https://www.npmjs.com/package/feascript
Should I do anything more?

@nikoscham

Copy link
Copy Markdown
Member

I haven't merged it with main branch yet, so the CDN links may not working. I will do it soon.

nikoscham added a commit that referenced this pull request May 22, 2025
* Made the initial build for npm (#25)
* Made the initial build for npm
* fix: update package.json with correct license, description and module type
- Change license from AGPLv3 to MIT to match actual project license
- Replace HTML image tag with proper text description
- Update type to "module" to align with ES module syntax
* Prepare package for npm publishing and bump version to 0.1.1
* Update package.json
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Update rollup.config.js
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Update rollup.config.js
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* fix: update test script to use jest and add jest as a devDependency
* fix: remove (still) unused wasm plugin from rollup configuration
* fix: remove unused rollup-plugin-wasm from devDependencies
* fix: revert test script to no tests configured and remove jest from devDependencies
* Add plotly.js as a peer and dev dependency; update Rollup config for external globals
* fix: remove unused rollup-plugin-typescript2 from configuration
* Update README.md
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* fix: update import paths in README for consistency with distribution bundle
* fix: correct parameter syntax in addBoundaryCondition example
---------
Co-authored-by: Sridhar.Mani <sridhar.mani@novacastindia.com>
Co-authored-by: Nikos Chamakos <nikoscham@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Update author and contributors information in package.json
* Add publishConfig to package.json for public access
---------
Co-authored-by: sridhar <2019309038@student.annauniv.edu>
Co-authored-by: Sridhar.Mani <sridhar.mani@novacastindia.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@sridhar-mani

Copy link
Copy Markdown
CollaboratorAuthor

Okay i have started the gpu methods. Have been trying to get some nice spot between the usage.

@nikoscham

Copy link
Copy Markdown
Member

Do you have any problem if I delete the merged branch (npm-release) @sridhar-mani ?

@sridhar-mani

Copy link
Copy Markdown
CollaboratorAuthor

I don't mind. But we could use two build releases right one for the git and another for npm which will consist only major version changes?

@nikoscham

Copy link
Copy Markdown
Member

I don't mind. But we could use two build releases right one for the git and another for npm which will consist only major version changes?

I see your point @sridhar-mani , but I’d prefer to keep a single, universal release process for both git and npm. Maintaining two separate release flows might get a bit confusing over time.

@sridhar-mani

Copy link
Copy Markdown
CollaboratorAuthor

Okay, then no problem. we can delete the branch.

@nikoscham
nikoscham deleted the npm-release branch May 26, 2025 10:58
@nikoschamnikoscham linked an issue Oct 7, 2025 that may be closed by this pull request
@nikoschamnikoscham added the enhancement New feature or request label Oct 7, 2025
@nikoschamnikoscham self-assigned this Oct 7, 2025
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancementNew feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Package FEAScript as an npm library

4 participants

@sridhar-mani@nikoscham@SridharNCI