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

PR #1081 does not fully fix type errors #1083

Closed
garth opened this issue May 6, 2021 · 6 comments · Fixed by #1115
Closed

PR #1081 does not fully fix type errors #1083

garth opened this issue May 6, 2021 · 6 comments · Fixed by #1115
Assignees
Labels

Comments

@garth
Copy link

garth commented May 6, 2021

Type of issue: (check with "[x]")

- [ ] New feature request
- [x] Bug  
- [ ] Support request

Current behavior:

In the build output, after #1081 was merged, type files (.d.ts) are now separated, but the generated .js and .map are still distributed alongside the original .ts files, which in some cases still causes the .ts versions to be picked up and errors are raised.

Expected behavior:

.ts files are probably not needed for npm package? but at least the build output should be restructured so that .d.ts, .js and .map files are together and .ts are separated.

Steps to reproduce the issue:

Reference @alfresco/js-api in unit tests (jest) and the following error is raised.

    SyntaxError: node_modules/@alfresco/js-api/src/api-legacy/legacy.ts: Unexpected token, expected "{" (104:7)

      102 |  * @deprecated 3.0.0
      103 |  */
    > 104 | export namespace Legacy {
          |        ^
      105 |     export interface Activiti {
      106 |         alfrescoApi: AlfrescoApiActiviti;
      107 |         aboutApi: AboutApi;

      at Parser._raise (node_modules/@babel/parser/src/parser/error.js:134:45)
      at Parser.raiseWithData (node_modules/@babel/parser/src/parser/error.js:129:17)
      at Parser.raise (node_modules/@babel/parser/src/parser/error.js:78:17)
      at Parser.unexpected (node_modules/@babel/parser/src/parser/util.js:173:16)
      at Parser.parseExport (node_modules/@babel/parser/src/parser/statement.js:1760:16)
      at Parser.parseStatementContent (node_modules/@babel/parser/src/parser/statement.js:274:25)
      at Parser.parseStatement (node_modules/@babel/parser/src/parser/statement.js:176:17)
      at Parser.parseBlockOrModuleBlockBody (node_modules/@babel/parser/src/parser/statement.js:910:25)
      at Parser.parseBlockBody (node_modules/@babel/parser/src/parser/statement.js:886:10)
      at Parser.parseProgram (node_modules/@babel/parser/src/parser/statement.js:74:10)

Remove the legacy.ts file and the error is gone (though the next alfresco .ts file found will raise).

Node version (for build issues):

node: 15.14.0

New feature request:

@eromano
Copy link
Contributor

eromano commented May 7, 2021

Ts file should not be present I agree, I will give it a look

@eromano eromano added the bug label May 7, 2021
garth added a commit to garth/alfresco-js-api that referenced this issue May 18, 2021
@garth
Copy link
Author

garth commented May 24, 2021

Just tested with v4.4.0 and the package types are still broken. Since this is a critical blocking issue, but easy to fix (see #1110), we will republish until there is an official fix.

Update: In case someone else is having typescript errors with v4.4.0, this version has the necessary fix. If you are not having typescript issues, please use the official version.

@Den-dp
Copy link
Contributor

Den-dp commented May 26, 2021

Since the current fix #1113 (removing ts files) is another way of solving #1074, it looks like the changes from #1081 became unnecessary and might be reverted.

Compile-time file resolution should work fine when .js and .d.ts files are placed in the same folder, the problem was only when both .ts, .js and .d.ts are in the same folder.

@Den-dp
Copy link
Contributor

Den-dp commented May 26, 2021

@garth to unblock myself, I used the following postinstall script to remove all .ts (but not .d.ts) files:

package.json

"scripts": {
    "postinstall": "node tools/js-api-fix.js",
}

tools/js-api-fix.js

const glob = require('glob');
const fs = require('fs');

glob('node_modules/@alfresco/js-api/**/!(*.d).ts', (err, files) => {
    if (err) throw err;

    console.log(`Fix @alfresco/js-api package issue: https://github.com/Alfresco/alfresco-js-api/issues/1074`);
    console.log(`Remove ${files.length} *.ts files from directory configured only for typings`);

    files.forEach(file => {
        console.log(`remove "${file}"`);
        fs.unlinkSync(file);
    });
});

Hope it helps

@garth
Copy link
Author

garth commented May 26, 2021

@Den-dp, I ran a yarn linked local version for a week or so which was OK for local development, but that was only a very temporary solution. I am now using a variant of 4.4 (linked above) with #1110 PR included. So no need for any post install scripts and all typescript errors are gone.

@DenysVuika
Copy link
Contributor

Fixed with the latest builds

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.