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

filename like "[id].fs" created with different path #1936

Closed
Neftedollar opened this issue Nov 1, 2019 · 17 comments
Closed

filename like "[id].fs" created with different path #1936

Neftedollar opened this issue Nov 1, 2019 · 17 comments

Comments

@Neftedollar
Copy link
Contributor

Neftedollar commented Nov 1, 2019

Description

Hello!
I'm trying to use fable for next js
I'm using fable-splitter and fable-compiler packages.
There is a part of my App.fsproj

    <Compile Include="pages/post/[id].fs" />
    <Compile Include="pages/about.fs" />

package.json

{
  "scripts": {
     "fableBuild": "fable-splitter App.fsproj --allFiles -o ./"
  }
}

starting npm run fableBuild
output part:

fable: Compiled pages/about.fs
fable: Compiled pages/post/[id].fs

about.js created near the about.fs ./pages/about.js
And [id].js created with the path ./post/[id].js but expected ./pages/post/[id].js
Снимок экрана 2019-11-02 в 00 10 21

Why I need filenames like this

Repro code

https://github.com/Neftedollar/Nextjs-Fable-simple-app/tree/repro

Related information

  • Fable version: 2.4.9
  • Fable-splitter version: 2.1.11
  • Operating system: Mac OS X 10.15.1 (19B88)
@alfonsogarciacaro
Copy link
Member

Hmm, probably the file name is not important here. This is just because fable-splitter is just flattening the folder structure so it doesn't reflect exactly the hierarchy of the original files. We did this originally to avoid path conflicts when compiling files coming from different hierarchies.

@ncave I think you mentioned once that you had solved this problem for fable-compiler-js. Would it be possible to port the solution to fable-splitter?

@Neftedollar Neftedollar changed the title filename like "[id].fs" creates with different path filename like "[id].fs" created with different path Nov 2, 2019
@ncave
Copy link
Collaborator

ncave commented Nov 2, 2019

@alfonsogarciacaro It should be possible, it's all this calling that.

We can easily add support in fable-compiler-js for PackageReference elements in projects (e.g. to resolve the Fable.React dependency, etc.). We can either consume the obj/project.assets.json created by dotnet restore, or (assuming package's already been restored), do an educated guess at the nuget path to it:

  • Binary at: ~/.nuget/packages/Fable.React/5.3.5/lib/netstandard2.0/Fable.React.dll, and the source code for it at ~/.nuget/packages/Fable.React/5.3.5/fable.

This requires the exact version to be specified in the reference (technically even wildcards can be supported, at the cost of adding complexity).

Of course, other package managers can be supported too, if we find a good mechanism to define that in the project file (but I'm not trying to reopen the package manager flame wars, so fnugget about it).

@Neftedollar
Copy link
Contributor Author

So, can I help somehow?
@alfonsogarciacaro @ncave

@alfonsogarciacaro
Copy link
Member

It'd be great if you could send a PR ti fix this @Neftedollar. This is the code in fable-splitter that decides the output path:

// TODO: implement better folder structure
function getOutPath(path: string, info: CompilationInfo): string {
let outPath = info.mapInOutPaths.get(path);
if (!outPath) {
// get file name without extensions
const fileName = Path.basename(path).replace(FSHARP_EXT, "").replace(JAVASCRIPT_EXT, "");
// flat folder structure (one level deep)
const pathDir = Path.dirname(path);
// If pathDir is same as entry dir don't create nested folder
const newPath = pathDir === Path.dirname(info.entry)
? fileName : Path.basename(pathDir) + "/" + fileName;
// dedup output path
let i = 0;
outPath = newPath;
// In Windows and Mac file paths are case insensitive
// so it may happen we get two identical paths with different case
while (info.dedupOutPaths.has(outPath.toLowerCase())) {
outPath = `${newPath}.${++i}`;
}
info.dedupOutPaths.add(outPath.toLowerCase());
info.mapInOutPaths.set(path, outPath);
}
return outPath;
}

You can keep the folder structure by doing something like Path.relative(Path.dirname(info.entry), path). The challenge would be files that are outside the .fsproj hierarchy (you can use @ncave code for example to see how to deal with files from fable library or packages, for example). I assume the current code for deduplicating file names in case of conflict should work as is.

@Neftedollar
Copy link
Contributor Author

@alfonsogarciacaro ok, I'll take it.
Thnx

@Neftedollar
Copy link
Contributor Author

@alfonsogarciacaro how can I build it?
I've started npm install at the root and then:

  fable-splitter git:(master) npm run build                         

> [email protected] build /Users/neftedollar/dev/fable/Fable/src/fable-splitter
> node build.js

exec /Users/neftedollar/dev/fable/Fable/node_modules/.bin/tsc 
../../node_modules/typescript/lib/lib.es2015.iterable.d.ts(41,6): error TS2300: Duplicate identifier 'IteratorResult'.
node_modules/@types/node/index.d.ts(179,11): error TS2300: Duplicate identifier 'IteratorResult'.
Error: tsc failed with code 1
    at runNpmBin (/Users/neftedollar/dev/fable/Fable/src/fable-splitter/build.js:23:15)
    at process._tickCallback (internal/process/next_tick.js:68:7)
    at Function.Module.runMain (internal/modules/cjs/loader.js:744:11)
    at startup (internal/bootstrap/node.js:285:19)
    at bootstrapNodeJSCore (internal/bootstrap/node.js:739:3)
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] build: `node build.js`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the [email protected] build script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

what I'm doing wrong?

@Neftedollar
Copy link
Contributor Author

@ncave @alfonsogarciacaro pls help #1936 (comment)

@ncave
Copy link
Collaborator

ncave commented Nov 19, 2019

@Neftedollar I'm not sure, normally building the splitter involves running npm run build splitter at the Fable root (all the build targets are in build.fsx).

@ncave
Copy link
Collaborator

ncave commented Nov 19, 2019

@Neftedollar But if you're stuck, I can take a look at adding this functionality in fable-splitter.
Also, you will soon have the option to just use the fable-compiler-js instead, once #1943 is accepted.

@Neftedollar
Copy link
Contributor Author

@ncave thnx i'll try to build it from the root. thnx

@Neftedollar
Copy link
Contributor Author

@ncave same story = /

node_modules/typescript/lib/lib.es2015.iterable.d.ts:41:6 - error TS2300: Duplicate identifier 'IteratorResult'.

41 type IteratorResult<T, TReturn = any> = IteratorYieldResult<T> | IteratorReturnResult<TReturn>;
        ~~~~~~~~~~~~~~

  src/fable-splitter/node_modules/@types/node/index.d.ts:179:11
    179 interface IteratorResult<T> { }
                  ~~~~~~~~~~~~~~
    'IteratorResult' was also declared here.

can you build splitter now with current package-lock.json?

here microsoft/TypeScript#32333 they said that we need just to update @type/node but nom update is not working for me (microsoft/TypeScript#32333 (comment)) and few days ago it comes back

@ncave
Copy link
Collaborator

ncave commented Nov 19, 2019

@Neftedollar Yes, it works for me (Node.js 12.x), and in CI. Perhaps the node/npm version that you're using is different, but that's probably not it.

@Neftedollar
Copy link
Contributor Author

Neftedollar commented Nov 19, 2019

@ncave so, I've asked two coworkers run npm run build splitter but they're all failed with the same error about IteratorResult.
BUT!
I've found that if I update @types/node now, it's fixed, there is new @types/node package was released.
So it's build now! I'll update package-lock.json with @types/node
TL;DR It's work thnx

@alfonsogarciacaro
Copy link
Member

Would it be possible to check if this is still an issue when using Fable 3 instead of Fable 2 + splitter?

@Neftedollar
Copy link
Contributor Author

@alfonsogarciacaro I'll check it this week

@Neftedollar
Copy link
Contributor Author

@alfonsogarciacaro I've checked it!
JS files stored in the same path as fs files.
image

So we can close this issue.

@alfonsogarciacaro
Copy link
Member

Awesome, thanks a lot for checking!

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

No branches or pull requests

3 participants