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

code splitting broken on Windows #169

Closed
igisev opened this issue Sep 4, 2019 · 6 comments · Fixed by #251
Closed

code splitting broken on Windows #169

igisev opened this issue Sep 4, 2019 · 6 comments · Fixed by #251
Labels
kind: bug Something isn't working properly priority: blocked Progress on this is blocked problem: stale Issue has not been responded to in some time scope: upstream Issue in upstream dependency solution: workaround available There is a workaround available for this issue topic: OS separators Path normalization between OSes. Windows = '\', POSIX = '/'. Rollup resolve = native, TS = POSIX

Comments

@igisev
Copy link

igisev commented Sep 4, 2019

What happens and why it is wrong

Briefly:
Code splitting is not supported now

Versions

  • typescript: 3.5.3
  • rollup: 1.20.3
  • rollup-plugin-typescript2: 0.24.0

rollup.config.js

import resolve from 'rollup-plugin-node-resolve';
import ts2 from 'rollup-plugin-typescript2';

export default {
  // NOTE: object here
  input: {
    'my-lib': 'src/main.ts',
    'my-tools': 'src/tools.ts'
  },
  output: {
    dir: 'dist',
    format: 'esm'
  },
  plugins: [
    resolve(),
    ts2()
  ]
};

tsconfig.json

{
  "compilerOptions": {
    "target": "esnext",
    "module": "esnext",
    "strict": true,
    "sourceMap": true,
    "importHelpers": true,
    "downlevelIteration": true,
    "moduleResolution": "node",
    "experimentalDecorators": true,
    "allowSyntheticDefaultImports": true,
    "esModuleInterop": true,

    "lib": ["esnext"],
    "types": ["node"]
  },
  "include": ["src/**/*.ts"],
  "exclude": ["node_modules"]
}

files

src/tools.ts

export function myFn() {
    console.log('Hello World!');
}

src/main.ts

import { myFn } from './tools';

console.log(myFn());

plugin output

dist/my-tools.js OK

function myFn() {
    console.log('Hello World!');
}

export { myFn };

dist/my-lib.js WRONG

function myFn() {
    console.log('Hello World!');
}

console.log(myFn());

expected output

dist/my-lib.js

import { myFn } from './my-tools.js';

console.log(myFn());
@ezolenko
Copy link
Owner

ezolenko commented Sep 4, 2019

Could you try the same thing with plain rollup, is it supposed to work? (turn ts into js manually and remove rpt2 from plugins)

@igisev
Copy link
Author

igisev commented Sep 5, 2019

Could you try the same thing with plain rollup, is it supposed to work?

This is a working well-tested feature in Rollup.
What is the point of checking it?
Rollup has a boilerplate: https://github.com/rollup/rollup-starter-code-splitting

Ok
case1:

  • rollup-plugin-typescript2 is off
  • all files renamed from ts to js

case2:

  • no plugins at all
  • all files renamed from ts to js

Result: output for case1 and case2 is identical (see below)

rollup.config.js

import resolve from 'rollup-plugin-node-resolve';

export default {
  // NOTE: object here
  input: {
    'my-lib': 'src/main.js',
    'my-tools': 'src/tools.js'
  },
  output: {
    dir: 'dist',
    format: 'esm'
  },
  plugins: [
    resolve()
  ]
};

output

dist/my-tools.js

function myFn() {
  console.log('Hello World!');
}

export { myFn };

dist/my-lib.js

import { myFn } from './my-tools.js';

console.log(myFn());

@ezolenko ezolenko added the kind: bug Something isn't working properly label Sep 5, 2019
@ezolenko
Copy link
Owner

ezolenko commented Sep 5, 2019

I see the problem, rollup doesn't normalize paths when using them as keys for code splitting. Workaround is to add rollupCommonJSResolveHack: true to rpt2 options (solves a similar problem with another plugin, they fixed it in recent release actually).

I'll open rollup bug.

@igisev
Copy link
Author

igisev commented Nov 12, 2019

It seems rollup is not going to fix.
@ezolenko, please. Can you solve this problem yourself?
This feature is critical for us.

@ezolenko
Copy link
Owner

@igisev can you use rollupCommonJSResolveHack: true as a workaround?

@agilgur5 agilgur5 added solution: workaround available There is a workaround available for this issue scope: upstream Issue in upstream dependency labels Apr 24, 2022
@agilgur5 agilgur5 added the topic: OS separators Path normalization between OSes. Windows = '\', POSIX = '/'. Rollup resolve = native, TS = POSIX label Jun 23, 2022
@agilgur5 agilgur5 changed the title code splitting support code splitting support on Windows Jun 23, 2022
@agilgur5 agilgur5 added the problem: stale Issue has not been responded to in some time label Jun 23, 2022
@agilgur5 agilgur5 changed the title code splitting support on Windows code splitting broken on Windows Jun 23, 2022
@agilgur5
Copy link
Collaborator

So this should have been fixed by #251, which was released in 0.30.0.

OP never responded regarding using rollupCommonJSResolveHack as a workaround, but #251 seems to state the same problem and basically makes the behavior of rollupCommonJSResolveHack always on.

So going to close this out as fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug Something isn't working properly priority: blocked Progress on this is blocked problem: stale Issue has not been responded to in some time scope: upstream Issue in upstream dependency solution: workaround available There is a workaround available for this issue topic: OS separators Path normalization between OSes. Windows = '\', POSIX = '/'. Rollup resolve = native, TS = POSIX
Projects
None yet
3 participants