jspm / npm

NPM Location Service
19 stars 34 forks source link

Support npm install hook scripts #36

Closed guybedford closed 8 years ago

guybedford commented 9 years ago

I wonder how well this will work?

trusktr commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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

trusktr commented 9 years ago

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

trusktr commented 9 years ago

https://github.com/jspm/jspm-cli/issues/377 shows how this feature (and rough example https://github.com/jspm/jspm-cli/pull/372) combined with code-based config could be nice.

guybedford commented 9 years ago

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

trusktr commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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

jpray commented 9 years ago

+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 commented 9 years ago

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 https://github.com/jspm/npm/issues/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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago

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 commented 9 years ago

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 https://github.com/jspm/jspm-cli/issues/916.

ghost commented 9 years ago

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 commented 8 years ago

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 commented 8 years ago

@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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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

unional commented 8 years ago

I have a case :smile::

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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

guybedford commented 8 years ago

Created a proposal at https://github.com/jspm/jspm-cli/issues/1899.

unional commented 8 years ago

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

unional commented 8 years ago

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

frederikschubert commented 8 years ago

@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();