[Web] NPM package include ts files in the output

rchiodo commented 1 year ago

Describe the issue

I'm trying to use onnxruntime-web in our build and I get a whole bunch of errors when webpacking because it's pulling .ts files from onnxruntime-web.

See this issue that @jakebailey originally filed. https://github.com/microsoft/onnxruntime/issues/7949 This issue is still reproing.

Here's an example error:

../pylance-internal/node_modules/onnxruntime-web/lib/onnxjs/backends/webgl/texture-manager.ts 29:38-50
[tsl] ERROR in C:\Users\aku91\source\repos\pyrx\packages\pylance-internal\node_modules\onnxruntime-web\lib\onnxjs\backends\webgl\texture-manager.ts(29,39)
      TS2304: Cannot find name 'WebGLTexture'.

To reproduce

npm install onnxruntime-web

Create a typescript file like so:

import * as ort from 'onnxruntime-web'

Build the typescript file.

Urgency

No response

ONNX Runtime Installation

Released Package

ONNX Runtime Version or Commit ID

1.13

Execution Provider

WASM

fs-eire commented 1 year ago

I am trying to find why the typescript files are being compiled. This is not the expected behavior. By default, ts-loader will not compile .ts files in node_modules.

Could you go to file node_modules/onnxruntime-web/package.json and remove line "module": "./lib/index.js", and build your project again, to see if the error still there?

rchiodo commented 1 year ago

Yes, that does fix the issue.

Removing the module from the package.json lets me import without any build errors.

Why do you ship the .ts files? Of all of the node_modules we use, yours is the only one that ships .ts files.

rchiodo commented 1 year ago

The change to the package.json fixes the build issue, but I'm getting runtime issues trying to use it though. Not sure what the problem is yet, but this happens trying to just load the onnxwebtime module:

[Error - 10:24:03 AM] (13728) Failed to load ONNX runtime. Exception ReferenceError: BUILD_DEFS is not defined
    at ../pylance-internal/node_modules/onnxruntime-web/lib/index.js (c:\Users\aku91\source\repos\pyrx\packages\vscode-pylance\dist\vendor.bundle.js:9663:1)
    at __webpack_require__ (c:\Users\aku91\source\repos\pyrx\packages\vscode-pylance\dist\server.bundle.js:11083:42)
    at DeepLearning.initialize (c:\Users\aku91\source\repos\pyrx\packages\vscode-pylance\dist\server.bundle.js:2833:30)
    at IntelliCodeCompletionListExtension._loadModel (c:\Users\aku91\source\repos\pyrx\packages\vscode-pylance\dist\server.bundle.js:3398:38)
    at async IntelliCodeCompletionListExtension.updateSettings (c:\Users\aku91\source\repos\pyrx\packages\vscode-pylance\dist\server.bundle.js:3235:13)

rchiodo commented 1 year ago

With the package.json change I can get it to work, if I do a few other tweaks:

Add this entry to my webpack:

    new webpack.DefinePlugin({
        BUILD_DEFS: {
            DISABLE_WEBGL: true,
            DISABLE_WASM: false,
            DISABLE_WASM_THREAD: true,
        },
    }),

Modify proxy-wrapper.js so that its require is consumable by webpack. I did this with a string replace of worker-loader?inline=no-fallback!./proxy-worker/main with ./proxy-worker/main.

Add a line to my ts-loader so that proxy-worker/main will build (skip reporting errors for it):

                    reportFiles: ['./**/*.{ts,js}', '!./node_modules/**/onnxruntime-web/*'],

fs-eire commented 1 year ago

The change to the package.json fixes the build issue, but I'm getting runtime issues trying to use it though. Not sure what the problem is yet, but this happens trying to just load the onnxwebtime module:

[Error - 10:24:03 AM] (13728) Failed to load ONNX runtime. Exception ReferenceError: BUILD_DEFS is not defined
    at ../pylance-internal/node_modules/onnxruntime-web/lib/index.js (c:\Users\aku91\source\repos\pyrx\packages\vscode-pylance\dist\vendor.bundle.js:9663:1)
    at __webpack_require__ (c:\Users\aku91\source\repos\pyrx\packages\vscode-pylance\dist\server.bundle.js:11083:42)
    at DeepLearning.initialize (c:\Users\aku91\source\repos\pyrx\packages\vscode-pylance\dist\server.bundle.js:2833:30)
    at IntelliCodeCompletionListExtension._loadModel (c:\Users\aku91\source\repos\pyrx\packages\vscode-pylance\dist\server.bundle.js:3398:38)
    at async IntelliCodeCompletionListExtension.updateSettings (c:\Users\aku91\source\repos\pyrx\packages\vscode-pylance\dist\server.bundle.js:3235:13)

It looks like that by removing "module": "./lib/index.js",, ts-loader no longer scan the ts files, but webpack still looks for file /lib/index.js. could you share me the webpack config file?

fs-eire commented 1 year ago

Yes, that does fix the issue.

Removing the module from the package.json lets me import without any build errors.

Why do you ship the .ts files? Of all of the node_modules we use, yours is the only one that ships .ts files.

the reason of shipping with the .ts files is because we want to enable debugging - we also ship the source map files. we don't expect ts-loader or webpack to pick/compile those files, and I will try if this can be fixed by updating some fields in package.json

rchiodo commented 1 year ago

Here's the entire contents of my webpack that gets it to build:

const webpack = require('webpack');
const path = require('path');
const CopyPlugin = require('copy-webpack-plugin');
const { WebpackDeduplicationPlugin } = require('../../build/webpack-deduplication-plugin');
const { cacheConfig, monorepoResourceNameMapper, tsconfigResolveAliases } = require('../pyright/build/lib/webpack');

const outPath = path.resolve(__dirname, 'dist');
const packages = path.resolve(__dirname, '..');

const typeshedFallback = path.resolve(packages, 'pyright', 'packages', 'pyright-internal', 'typeshed-fallback');
const bundled = path.resolve(packages, 'pylance-internal', 'bundled');
const schemas = path.resolve(packages, 'pyright', 'packages', 'vscode-pyright', 'schemas');
const scripts = path.resolve(packages, 'pylance-internal', 'scripts');
const browserProcess = path.resolve(packages, 'pylance-internal', 'src', 'common', 'browserProcess.ts');

const { distDir: onnxDir } = require('../pylance-internal/build/findonnx');

class PylanceManifestPlugin {
    /**
     * @param {webpack.Compiler} compiler
     */
    apply(compiler) {
        const hookOptions = {
            name: 'PylanceManifestPlugin',
            stage: Infinity,
        };

        compiler.hooks.thisCompilation.tap(hookOptions, (compilation) => {
            compilation.hooks.processAssets.tap(hookOptions, (assets) => {
                const files = Object.keys(assets);

                // Manually add files added by browserConfig; keep synced.
                files.push('browser.server.js', 'browser.extension.js');

                // TODO: Include sizes? Get sizes for browser assets?
                const manifest = {
                    files,
                };

                compilation.emitAsset('folderIndex.json', new webpack.sources.RawSource(JSON.stringify(manifest)));
            });
        });
    }
}

/**@type {(env: any, argv: { mode: 'production' | 'development' | 'none' }) => import('webpack').Configuration}*/
const nodeConfig = (_, { mode }) => {
    return {
        context: __dirname,
        entry: {
            extension: './src/extension.ts',
            server: './src/server.ts',
        },
        target: 'node',
        output: {
            filename: '[name].js',
            path: outPath,
            libraryTarget: 'commonjs2',
            devtoolModuleFilenameTemplate:
                mode === 'development' ? '../[resource-path]' : monorepoResourceNameMapper('pylance-client'),
            clean: {
                keep: /^browser/, // Ignore the output of the browser config below.
            },
        },
        devtool: 'source-map',
        cache: mode === 'development' ? cacheConfig(__dirname, __filename, `node-${mode}`) : undefined,
        stats: {
            all: false,
            errors: true,
            warnings: true,
            publicPath: true,
            timings: true,
        },
        resolve: {
            extensions: ['.js', '.ts'],
            alias: tsconfigResolveAliases('tsconfig.json'),
        },
        externals: {
            vscode: 'commonjs vscode',
            fsevents: 'commonjs2 fsevents',
        },
        module: {
            rules: [
                {
                    test: /\.ts$/,
                    loader: 'ts-loader',
                    options: {
                        configFile: 'tsconfig.json',
                        allowTsInNodeModules: false,
                        reportFiles: ['./**/*.{ts,js}', '!./node_modules/**/onnxruntime-web/*'],
                    },
                },
            ],
        },
        plugins: [
            new WebpackDeduplicationPlugin(),
            new CopyPlugin({
                patterns: [
                    { from: `${onnxDir}/ort-wasm.wasm`, to: '[name][ext]' },
                    { from: typeshedFallback, to: 'typeshed-fallback' },
                    { from: bundled, to: 'bundled' },
                    { from: schemas, to: 'schemas' },
                    { from: scripts, to: 'scripts' },
                ],
            }),
            new PylanceManifestPlugin(),
            new webpack.DefinePlugin({
                BUILD_DEFS: {
                    DISABLE_WEBGL: true,
                    DISABLE_WASM: false,
                    DISABLE_WASM_THREAD: true,
                },
            }),
        ],
    };
};

/**@type {(env: any, argv: { mode: 'production' | 'development' | 'none' }) => import('webpack').Configuration}*/
const browserConfig = (_, { mode }) => {
    return {
        context: __dirname,
        entry: {
            extension: {
                import: './src/browser/extension.ts',
                library: {
                    type: 'commonjs2',
                },
            },
            server: {
                import: './src/browser/server.ts',
                // The server is loaded in a Worker and is not a library. Attempting
                // to export (e.g. via commonjs exports) will crash.
            },
            'tests/web/index': {
                import: './src/tests/web/index.ts', // For web tests
                library: {
                    type: 'commonjs2',
                },
            },
        },
        target: 'webworker',
        output: {
            filename: 'browser.[name].js',
            path: outPath,
            devtoolModuleFilenameTemplate:
                // Use absolute paths, as when run in vscode.dev, we're not running inside of the pyrx repo.
                mode === 'development' ? '[absolute-resource-path]' : monorepoResourceNameMapper('pylance-client'),
            clean: {
                keep: /^(?!browser)/, // Ignore non-browser files.
            },
        },
        devtool: 'source-map',
        cache: mode === 'development' ? cacheConfig(__dirname, __filename, `browser-${mode}`) : undefined,
        stats: {
            all: false,
            errors: true,
            warnings: true,
            publicPath: true,
            timings: true,
        },
        resolve: {
            extensions: ['.ts', '.js'],
            alias: tsconfigResolveAliases('tsconfig.json'),
            fallback: {
                buffer: require.resolve('buffer/'), // Used by stream
                events: require.resolve('events/'), // Used by stream
                stream: require.resolve('stream-browserify'),
                path: require.resolve('path-browserify'),
                process: browserProcess,
                // Note: this will make these imports empty objects, not make them fail to resolve.
                crypto: false,
                worker_threads: false,
                child_process: false,
                v8: false,
            },
        },
        externals: {
            vscode: 'commonjs vscode',
        },
        module: {
            rules: [
                {
                    test: /\.ts$/,
                    loader: 'ts-loader',
                    options: {
                        configFile: 'tsconfig.json',
                    },
                },
            ],
        },
        plugins: [
            new WebpackDeduplicationPlugin(),
            new webpack.ProvidePlugin({
                process: browserProcess,
            }),
        ],
    };
};

module.exports = [nodeConfig, browserConfig];

jakebailey commented 1 year ago

the reason of shipping with the .ts files is because we want to enable debugging - we also ship the source map files. we don't expect ts-loader or webpack to pick/compile those files, and I will try if this can be fixed by updating some fields in package.json

FWIW the solution here if you really, really want to ship the TS source is to use outDir and have a src dir and a dist dir, such that no downstream code can accidentally attempt to use the TS code.

And if you're wanting downstream users to use the TS source for anything else (including better downleveling), that's a bad idea in general.

fs-eire commented 11 months ago

In latest main branch, we use the "exports" field in package.json to define entries for variance scenarios of importing and mainstream bundlers should be able to work with it. Currently supporting IIFE/CommonJS/ESM imports and bundlers will not look at ./lib/index.ts now.

microsoft / onnxruntime