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

(feat): Add build command by wrapping rollup #2544

Merged
merged 10 commits into from
Oct 24, 2023

Conversation

JayaKrishnaNamburu
Copy link
Member

@JayaKrishnaNamburu JayaKrishnaNamburu commented Sep 22, 2023

This PR aims to introduce the build command. Which will work in 2 modes. Basically the command wraps the rollup Node API with some defaults to run in simple mode. Where, we define some standard defaults and works with some projects out of the box.

jspm build --entry app.js --output build/

Then there is an another mode, where users can extend the behaviour with custom rollup setups. And the wrapper loads the config from the URL and starts the build.

jspm build --build-config build.mjs

is a simple rollup file. Which can have any defaults in it. Example

// build.mjs
import { defineConfig } from "rollup";
import banner from "rollup-plugin-banner";
import { babel } from "@rollup/plugin-babel";
import { nodeResolve } from "@rollup/plugin-node-resolve";

export default defineConfig({
  input: "./app.js",
  plugins: [
    nodeResolve(),
    babel({
      presets: ["@babel/preset-react"],
    }),
    banner.default("Running rolup with a wrapper around with jspm cli"),
  ],
  output: {
    file: "./build.js",
  },
});

Which generates a output of

// Running rolup with a wrapper around with jspm cli
import react from 'react';

console.log('This is from utils file');

console.log(react);
const App = () => {
  return /*#__PURE__*/React.createElement("div", null, "Hello World");
};

export { App };

The importmap.json processing is not handled yet. But once done, it can bundle react from jspm CDN and include in the build.

@JayaKrishnaNamburu JayaKrishnaNamburu self-assigned this Sep 22, 2023
@JayaKrishnaNamburu
Copy link
Member Author

Made some resolution changes with the rollup-plugin now. It can download the deps and bundle them too.
The command

jspm build --build-config build-config.mjs

downloads all the dependencies that are defined in the importmap.json and bundles them along.

@JayaKrishnaNamburu
Copy link
Member Author

Crazy how this wraps the whole vite-plugin-jspm functionality 😄

Copy link
Member

@guybedford guybedford left a comment

Choose a reason for hiding this comment

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

Excellent work, very exciting to see this one. Can you add some tests? Let me know if you need help getting them going.

chompfile.toml Show resolved Hide resolved
src/build/rollup-importmap-plugin.ts Outdated Show resolved Hide resolved
src/build/rollup-importmap-plugin.ts Show resolved Hide resolved
src/build/rollup-importmap-plugin.ts Outdated Show resolved Hide resolved
src/build/rollup-importmap-plugin.ts Show resolved Hide resolved
src/build/rollup-importmap-plugin.ts Outdated Show resolved Hide resolved
src/cli.ts Outdated Show resolved Hide resolved
src/build/index.ts Outdated Show resolved Hide resolved
@guybedford
Copy link
Member

Also documentation should be added in https://github.com/jspm/jspm-cli/tree/main/docs, which would then get added to the site.

@JayaKrishnaNamburu
Copy link
Member Author

JayaKrishnaNamburu commented Sep 25, 2023

Thanks for the review @guybedford, good to see that the PR was in the right direction 😄. Will make the necessary changes, and see how we can integrate this with other commands. Like we have traceInstall available with jspm installl that updates .html files.

But if users are bundling the deps too. We don't need to add importmap anymore. So, just trying to list use-cases like those. And see how we can streamline the workflows and connect these commands more together 👍

@JayaKrishnaNamburu JayaKrishnaNamburu marked this pull request as ready for review October 20, 2023 17:23
@JayaKrishnaNamburu
Copy link
Member Author

@guybedford made the final changes, and added the documentation for the same. Please check if need any changes/we can merge and release it.

Copy link
Member

@guybedford guybedford left a comment

Choose a reason for hiding this comment

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

Thanks for working on the feedback here. I've left another couple of change requests. With those I think we could then approve and land and release. Ping me on discord if you want to discuss further at all.

src/cli.ts Outdated Show resolved Hide resolved
src/cli.ts Outdated Show resolved Hide resolved
src/build/index.ts Outdated Show resolved Hide resolved
@JayaKrishnaNamburu
Copy link
Member Author

Done with the directory changes 👍

Copy link
Member

@guybedford guybedford left a comment

Choose a reason for hiding this comment

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

Thanks, looks good down to the externals handling versus resolution errors.

src/build/rollup-importmap-plugin.ts Show resolved Hide resolved
@JayaKrishnaNamburu
Copy link
Member Author

Should i release 3.1.0 with this change ?

@guybedford
Copy link
Member

Yes please!

@JayaKrishnaNamburu JayaKrishnaNamburu merged commit d77f468 into main Oct 24, 2023
4 checks passed
@JayaKrishnaNamburu JayaKrishnaNamburu deleted the feat/build-command branch October 24, 2023 03:23
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.

2 participants