Skip to content
This repository was archived by the owner on Feb 18, 2024. It is now read-only.

Support npm install hook scripts #36

Closed
guybedford opened this issue Jan 8, 2015 · 34 comments
Closed

Support npm install hook scripts #36

guybedford opened this issue Jan 8, 2015 · 34 comments

Comments

@guybedford
Copy link
Member

I wonder how well this will work?

@trusktr
Copy link

trusktr commented Jan 9, 2015

Well in my case I'd really like to have jspm-specific scripts, because with jspm there's no need to compile my ES6 to CJS using 6to5. So, my package .json would be like the following with a jspm-specific scripts section.

{
  "name": "infamous",
  "scripts": { // ran by npm
    "postinstall": "npm run build-cjs",
    "build-cjs": "./node_modules/.bin/6to5 --optional coreAliasing src --source-maps-inline --out-dir . --modules common"
  },
  "jspm": {
    "scripts": { // ran by jspm.
      "postinstall": "mv src/* ."
    }
  }
}

You're right about not being sure if jspm should run the scripts hooks. How about this: run scripts when they are found inside the jspm property and ignore root-level scripts, and root-level scripts if jspm.scripts is set to true (regardless of the endpoint used, npm: or github:). In the following example, the root-level scripts are evaluated.

// package.json
{
  "name": "infamous",
  "scripts": {
    "postinstall": "npm run build-cjs",
    "build-cjs": "./node_modules/.bin/6to5 --optional coreAliasing src --source-maps-inline --out-dir . --modules common"
  },
  "jspm": {
    "scripts": true
  }
}

The default value of jspm.scripts would be false if there is no jspm section in the package.json. So the following config

// package.json
{
  "name": "infamous",
  "scripts": {
    "postinstall": "npm run build-cjs",
    "build-cjs": "./node_modules/.bin/6to5 --optional coreAliasing src --source-maps-inline --out-dir . --modules common"
  }
}

would behave the same as

// package.json
{
  "name": "infamous",
  "scripts": {
    "postinstall": "npm run build-cjs",
    "build-cjs": "./node_modules/.bin/6to5 --optional coreAliasing src --source-maps-inline --out-dir . --modules common"
  },
  "jspm": {
    "scripts": false
  }
}

The logic roughly:

if (jspm.scripts === true) {
    run_root_level_scripts();
}
else if (jspm.scripts.constructor === Object) {
    run_jspm_section_scripts();
}

@trusktr
Copy link

trusktr commented Jan 9, 2015

Since dependencies with jspm are flat, all in a single directory, I think this flexibility would make sense because there could be scripts that fail when a node_modules directory can't be found, for example.

@trusktr
Copy link

trusktr commented Jan 9, 2015

Hmm, but also if scripts are supported in the jspm property, it would make sense to also support that for any type of endpoint, wouldn't it? The best thing would be to just let the final user of jspm decide when or when not to use scripts. :)

@guybedford
Copy link
Member Author

This issue is only tracking the support of scripts for packages installed from npm:*.

@trusktr
Copy link

trusktr commented Jan 10, 2015

True. I believe jspm users would like the freedom to run scripts regardless of endpoint 🎉, so I'm making that point here, and then, yeah, the result would need to be implemented over jspm-cli.

@trusktr
Copy link

trusktr commented Jan 10, 2015

jspm/jspm#377 shows how this feature (and rough example jspm/jspm#372) combined with code-based config could be nice.

@guybedford
Copy link
Member Author

@trusktr I've yet to see a use case this would be useful for in jspm.

@trusktr
Copy link

trusktr commented Jan 11, 2015

The answer is simple: for packages that don't follow npm or github specs, and need special handling. It can't be any clearer than that. SUre, I can manually run a script, but making it simply automated is not much harder, and more convenient.

@trusktr
Copy link

trusktr commented Jan 11, 2015

I have to manually run a bash script after jspm install. Why not the ability to run it automatically on postinstall? That would be so nice.

@trusktr
Copy link

trusktr commented Jan 11, 2015

Trying installing leaflet and using it in your browser. Can you make it run without touching jspm's auto-generated config.js and without running any scripts after jspm installing it?

@trusktr
Copy link

trusktr commented Jan 11, 2015

I meant, specifically try installing leaflet=github:RickMohr/Leaflet@better-inertial-scrolling.

@jpray
Copy link

jpray commented Mar 25, 2015

+1 for ability to define a jspm postinstall hook. In my case, I'm using jspm to manage various UI components that include JS and/or SASS. The JS part obviously works fine, but consuming the SASS partials doesn't work because SASS doesn't have a mechanism to resolve "@[version]" in the import paths. I realize SASS could be enhanced to work better with jspm, but a postinstall hook would allow me to create a workable solution for this now so that I can continue to use (and advocate for using) jspm within my team.

@geelen
Copy link

geelen commented Apr 26, 2015

I just hit this issue. Trying to work with cssnext, I hit a ton of trouble (the same that has been hitting me with autoprefixer in #52) but also a new one from the caniuse-api project. It has a post-install script that does this:

node -e "require('shelljs/global');if(test('-d', 'dist'))exec('node dist/generate-features.js')"

For me, trying to get JSPM to anticipate this kind of use case is impossible. It feels like making your code depend on a build artefact that you use a post-install script to generate (particularly one that explicitly refers to node on the command line) isn't a great idea. Particularly if that's the only thing preventing your code from executing within a browser...

@tengyifei
Copy link

This would be highly appreciated. We have a bunch of modules that uses a npm post install hook to compile scss files. Now I have to explicitly invoke a node command.

@theefer
Copy link

theefer commented Jul 5, 2015

I have just run into this while trying to use node-sass from JSPM. Its package.json has a postinstall script which checks out the libsass binaries needed by node-sass.

I am trying to get node-sass to work as part of the SystemJS Sass plugin I'm working on. It would obviously only used for bundling (i.e. JSPM running in a Node env), whereas in-browser use of the plugin already works using sass.js.

Is there any possible overrides or workarounds to get this to work?

@guybedford
Copy link
Member Author

@theefer I'm trying to avoid running postinstall scripts because I don't think a browser package manager should give full permissions for dependencies to the host on install for security.

It would be nice to approach the Node binary problem freshly in due course as well.

In the mean time, it's not an advisable best-practice, but you could use System._nodeRequire to load it only when bundling. Then rely on an npm to install for bundling support.

See https://github.com/systemjs/plugin-css/blob/master/css-builder.js#L4 and https://github.com/systemjs/plugin-css/blob/master/css.js#L83 for example patterns here.

@theefer
Copy link

theefer commented Jul 5, 2015

Thanks for the feedback, makes sense.

Then rely on an npm to install for bundling support.

I'm not sure what you mean by this? How/when would I trigger an npm install of node-sass for my Sass plugin to load?

@guybedford
Copy link
Member Author

@theefer with a message similar to https://github.com/systemjs/plugin-css/blob/master/css.js#L86 to make npm install node-sass part of the installation.

@theefer
Copy link

theefer commented Jul 5, 2015

Ah right that makes sense!

I can't seem to get System._nodeRequire("node-sass") to pick up node-sass from node_modules though:

/tmp/test $ ls node_modules/
node-sass
/tmp/test $ cat test.js 
var sass = System._nodeRequire('node-sass');
console.log("SASS", sass);

/tmp/test $ jspm run test.js 

err  Error: Cannot find module 'node-sass'
        Evaluating file:///tmp/test/test.js
        Error loading file:///tmp/test/test.js
seb@nagini /tmp/test $ jspm --version
0.16.0-beta.3
Running against global jspm install.

Being stupid I reckon? :)

@guybedford
Copy link
Member Author

No this is a bug - the node require is from the internals of the SystemJS package, when we want a require scoped to the jspm project itself. I've created jspm/jspm#916.

@ghost
Copy link

ghost commented Sep 18, 2015

Hi,

I've run into this issue with kerberos. It runs a node-gyp rebuild to build the operating system specific binaries via the "install" script which doesn't execute when I do a jspm install npm:kerberos

@guybedford I know you've said:

I don't think a browser package manager should give full permissions for dependencies to the host on install for security.

It would be nice to approach the Node binary problem freshly in due course as well.

But there are just some npm packages that won't work and weren't built to work without running the preinstall, postinstall, and install. It's commonly expected that those scripts, if present, will always run for npm packages on install.

In short, I believe to fully support npm packages the above mentioned scripts should be executed.

I was thinking it's ok to do it in the npm-endpoint. The on('end') event callback function for extracting the package, after download, can be used to run the install scripts. The below code works.

.on('end', function() {
    //Flag initialised in processPackageConfig() to indicate whether the install scripts are present
    if (self._runInstallScripts) {
       //Run preinstall, install, and post install scripts if present.
       asp(exec)('npm install', { cwd: targetDir }).then(function() {
            //Remove node modules after then install scripts have run
            return asp(exec)('rm -rf node_modules', { cwd: targetDir });
       }).then(function() {
            resolve();
       }).catch(function(error) {
            reject(error);
       })
     }
     else {
           resolve();
     }
});

Note the _runInstallScripts flag also needs to be added in the processPackageConfig:

  if (pjson.scripts && pjson.scripts.install) {
            this._runInstallScripts = true;
        }

The users of jspm don't need to be in control of the scripts installing because it's default behaviour for npm packages.

I'm willing to create a pull request.

I hope this is helpful.

@guybedford
Copy link
Member Author

jspm is a browser package manager, and is not trying to replace npm for server-side code execution. We will be expanding server code support more and more to cover universal javascript use cases, but supporting everything npm does is not currently a goal.

@chauthai
Copy link

@guybedford
I'm using JSPM in a large scale app and would really like to use post install hook to manipulate the config.js to alter alias mappings.

As shown in the diagram below, the standard implementation of the App depends on the Navi package and the Details package. These two package also depend on Map (Google).
In a customised version of the App I want to replace Map (Google) with Map (Bing) without touching the dependencies of packages Navi and Details.

The easiest solution would be to use a post install hook of JSPM to set the alias of Map to Map (Bing).
Without a hook my only solution is to wrap jspm install in gulp task and change the alias after the jspm installation finished, to guarantee my alias remapping doesn't get overwritten.

Do you have any other suggestions?

dep_mapping_jspm

@screendriver
Copy link

jspm is a browser package manager, and is not trying to replace npm for server-side code execution. We will be expanding server code support more and more to cover universal javascript use cases, but supporting everything npm does is not currently a goal.

Hi @guybedford,

but what we are talking about is not an explicit server feature. We need an install hook at build or install time at the command line when you run jspm install and/or jspm bundle. If we had an install hook we could manipulate and replace depending packages (see @chauthai's post).

@guybedford
Copy link
Member Author

I still haven't heard a compelling use case for such a hook. For example I think @chauthai's post is solved nicely by peer dependencies in the 0.17 jspm.

@screendriver
Copy link

Nice to hear. Do you have any example or documentation about that new feature in 0.17?

@unional
Copy link

unional commented Jun 10, 2016

I have a case 😄:

Able to run typings to install typings for jspm packages.

EDIT: Notice this is only for npm:. I think my use case need overall jspm support. @guybedford should I open an issue on jspm-cli?

@guybedford
Copy link
Member Author

For this use case, we need to find typings approaches in jspm based on module metadata and that don't require execution. Help paving this work is very welcome. Install hooks are a big security issue.

@frederikschubert
Copy link

frederikschubert commented Jun 10, 2016

I also have a use-case that has to do with typescript :) :
I would really like to have a jspm postinstall/-update hook. I use typescript in my projects and with the latest additions to plugin-typescript, type checking works seamlessly in the browser. But I also need type checking in the editor. For that I have to use typescript@next's path mappings in the tsconfig.json. Because the mappings have to be changed when the version of the dependency changes I have written a tool that reads the jspm.config.js and writes the right path mappings to the tsconfig.json. Now I want to run that tool after each jspm install or update.

@guybedford
Copy link
Member Author

I guess we could support npm-style "scripts" hooks but only for the local package.json file, and not installed packages.

@guybedford
Copy link
Member Author

Created a proposal at jspm/jspm#1899.

@unional
Copy link

unional commented Jun 10, 2016

@frederick, can you share your code? I need to do similar things for typings

@unional
Copy link

unional commented Jun 10, 2016

@guybedford I am working on the details and will include you to the discussion.

@frederikschubert
Copy link

@unional Yes but this is a pretty naive implementation ;-)

#! /usr/bin/env node

import * as path from "path";
import * as fs from "fs";
import * as System from "systemjs";

const PROCESS_WORKING_DIR = process.cwd();

const loadSystemConfig = () => {
    require(path.join(PROCESS_WORKING_DIR, "jspm.config.js"));
};

const loadTypescriptConfig = () => {
    return require(path.join(PROCESS_WORKING_DIR, "tsconfig.json"));
};

const loadPackage = () => {
    return require(path.join(PROCESS_WORKING_DIR, "package.json"));
};

const setTypescriptBaseUrl = (tsconfig: any, jspmPackagesDirectory: string) => {
    tsconfig.compilerOptions.baseUrl = tsconfig.compilerOptions.baseUrl || path.join(".", jspmPackagesDirectory);
};

const setTypescriptPaths = (tsconfig: any, paths: any) => {
    tsconfig.compilerOptions.paths = paths;
};

const saveTypescriptConfig = (tsconfig: any) => {
    fs.writeFileSync(path.join(PROCESS_WORKING_DIR, "tsconfig.json"), JSON.stringify(tsconfig, null, 2));
};

const getPackageMainFile = (dependencyName: string, packagePath: string) => {
    let packageMainFile;
    try {
        const dependencyPackage = require(packagePath);
        packageMainFile = dependencyPackage.main;
    } catch (error) {
        console.warn(`Dependency ${dependencyName} has no main entry in its package.json`);
    }
    return packageMainFile;
};

const removeJsSuffix = (path) => {
    if (path.endsWith(".js")) {
        path = path.substring(0, path.length - 3);
    }
    return path;
};

const sanitizeMainPath = (mainPath: string) => {
    let sanitizedMainPath = mainPath.replace("./", "");
    return removeJsSuffix(sanitizedMainPath);
};

const mapSystemConfigToTypescriptConfig = () => {

    loadSystemConfig();

    let tsconfig = loadTypescriptConfig();
    const pkg = loadPackage();

    const JSPM_PACKAGES_DIRECTORY = pkg.jspm && pkg.jspm.directories && pkg.jspm.directories.packages || "jspm_packages";
    const JSPM_PACKAGES_PATH = path.join(PROCESS_WORKING_DIR, JSPM_PACKAGES_DIRECTORY);

    const jspmDependencies = System.map;
    const jspmDependencyNames = Object.keys(jspmDependencies);

    let paths = {};

    jspmDependencyNames.forEach(dependencyName => {
        const dependencyPath = jspmDependencies[dependencyName].replace("npm:", "npm/").replace("github:", "github/");

        paths[`${dependencyName}*`] = [
            `${dependencyPath}*`,
            `${dependencyPath}/index`
        ];

        const dependencyMainFile = getPackageMainFile(dependencyName, path.join(JSPM_PACKAGES_PATH, dependencyPath, "package.json"));

        if (dependencyMainFile) {
            paths[`${dependencyName}*`].push(path.join(dependencyPath, sanitizeMainPath(dependencyMainFile)));
        }
    });
    setTypescriptBaseUrl(tsconfig, JSPM_PACKAGES_DIRECTORY);
    setTypescriptPaths(tsconfig, paths);

    saveTypescriptConfig(tsconfig);
};

mapSystemConfigToTypescriptConfig();

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants